Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove API Client from the tree #1007

Merged
merged 17 commits into from
May 7, 2020
Merged

Remove API Client from the tree #1007

merged 17 commits into from
May 7, 2020

Conversation

heyhippari
Copy link
Contributor

@heyhippari heyhippari commented Mar 31, 2020 •
edited
Loading

Changes

Branch to parallel my cleanup of the API Client in order for it to be removed from this repository.

The API Client side is at jellyfin-archive/jellyfin-apiclient-javascript#36

Issues

@heyhippari heyhippari changed the title Remove API Client from the tree [WIP] Remove API Client from the tree Mar 31, 2020
@dkanada
Copy link
Member

dkanada commented Mar 31, 2020

What a beautiful reduction in code 😄 can't wait to get this through.

@heyhippari heyhippari mentioned this pull request Mar 31, 2020
Merged
@ThibaultNocchi
Copy link
Member

ThibaultNocchi commented Apr 1, 2020 •
edited
Loading

Maybe you'll be able to help as I'm having problems with a "ReferenceError: ApiClient is not defined" error in controllers/wizard/start.js.

I'm using the latest server commit and replacing its web folder with the latest web client commit. I'm being normally redirected to the wizard and I'm hit with this error. And trying to get window.ApiClient from the JS console returns me an undefined value.

There is no problem when using the bundled web client. BTW, what version of the client am I building when building from the latest server commit? Thanks!

@heyhippari heyhippari marked this pull request as draft April 25, 2020 15:27
@heyhippari heyhippari changed the title [WIP] Remove API Client from the tree Remove API Client from the tree Apr 25, 2020
@heyhippari heyhippari added the es6 label Apr 25, 2020
@heyhippari heyhippari force-pushed the apiclient-be-gone branch from 2dd109a to 1766a8f Compare May 1, 2020 10:08
@heyhippari heyhippari marked this pull request as ready for review May 1, 2020 10:09
@heyhippari
Copy link
Contributor Author

@jellyfin/web This depends on jellyfin-archive/jellyfin-apiclient-javascript#38, which is under final cleanup.

ferferga
ferferga requested changes May 1, 2020
View reviewed changes
@heyhippari heyhippari added the blocked Requires work on the web client to finish label May 1, 2020
@heyhippari heyhippari removed the blocked Requires work on the web client to finish label May 1, 2020
@heyhippari heyhippari requested a review from ferferga May 1, 2020 12:19
dmitrylyzo
dmitrylyzo reviewed May 1, 2020
View reviewed changes
package.json Outdated
@@ -90,18 +91,19 @@
"test": [
"src/components/autoFocuser.js",
"src/components/cardbuilder/cardBuilder.js",
"src/scripts/dom.js",
"src/components/filedownloader.js",
"src/scripts/filesystem.js",
"src/scripts/keyboardnavigation.js",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dup

Suggested change
"src/scripts/keyboardnavigation.js",

@heyhippari heyhippari mentioned this pull request May 1, 2020
Merged
2 tasks
@heyhippari heyhippari mentioned this pull request May 2, 2020
Merged
3 tasks
@heyhippari heyhippari force-pushed the apiclient-be-gone branch from 81d0010 to 8e337da Compare May 2, 2020 19:43
@heyhippari
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@heyhippari heyhippari requested review from dkanada and EraYaN as code owners May 4, 2020 08:43
JustAMan
JustAMan approved these changes May 4, 2020
View reviewed changes
@heyhippari heyhippari force-pushed the apiclient-be-gone branch from 01260ce to 1bfe501 Compare May 5, 2020 09:59
@JustAMan JustAMan added the merge conflict Conflicts prevent merging label May 6, 2020
@heyhippari heyhippari force-pushed the apiclient-be-gone branch 2 times, most recently from ca3c789 to bff66fb Compare May 7, 2020 10:37
@heyhippari
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@heyhippari heyhippari marked this pull request as draft May 7, 2020 11:37
@heyhippari heyhippari force-pushed the apiclient-be-gone branch from bff66fb to bab96ae Compare May 7, 2020 14:36
@heyhippari heyhippari marked this pull request as ready for review May 7, 2020 14:36
@heyhippari heyhippari force-pushed the apiclient-be-gone branch from 76226e9 to ca4d60c Compare May 7, 2020 14:50
@heyhippari heyhippari requested a review from a team May 7, 2020 14:56
@sonarqubecloud
Copy link

sonarqubecloud bot commented May 7, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 1 Code Smell

No Coverage information No Coverage information
9.9% 9.9% Duplication

JustAMan
JustAMan reviewed May 7, 2020
View reviewed changes
JustAMan
JustAMan reviewed May 7, 2020
View reviewed changes
@heyhippari heyhippari removed the merge conflict Conflicts prevent merging label May 7, 2020
thornbill
thornbill approved these changes May 7, 2020
View reviewed changes
Copy link
Member

@thornbill thornbill left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Seems to be working as expected!

@dkanada dkanada merged commit a7241d3 into jellyfin:master May 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dmitrylyzo dmitrylyzo dmitrylyzo left review comments

@JustAMan JustAMan JustAMan approved these changes

@thornbill thornbill thornbill approved these changes

@dkanada dkanada dkanada approved these changes

@ferferga ferferga Awaiting requested review from ferferga

@EraYaN EraYaN Awaiting requested review from EraYaN

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@heyhippari @dkanada @ThibaultNocchi @JustAMan @thornbill @ferferga @dmitrylyzo