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

Upgrade @nextcloud/eslint-config to 7.0.2 #2594

Merged
merged 2 commits into from
Apr 8, 2022
Merged

Upgrade @nextcloud/eslint-config to 7.0.2 #2594

merged 2 commits into from
Apr 8, 2022

Conversation

raimund-schluessler
Copy link
Contributor

@raimund-schluessler raimund-schluessler commented Apr 1, 2022 •
edited
Loading

This updates @nextcloud/eslint-config to the latest version and fixes most lint issues introduced by it.

@raimund-schluessler raimund-schluessler added 2. developing Work in progress dependencies Pull requests that update a dependency file feature: app-sidebar Related to the app-sidebar component technical debt breaking PR that requires a new major version feature: emoji picker Related to the emoji picker component labels Apr 1, 2022
@raimund-schluessler raimund-schluessler force-pushed the dependencies/eslint-config branch 2 times, most recently from a37bd18 to f81a14a Compare April 1, 2022 21:49
@raimund-schluessler raimund-schluessler added this to the 6.0.0 milestone Apr 2, 2022
@raimund-schluessler raimund-schluessler mentioned this pull request Apr 2, 2022
Merged
@raimund-schluessler raimund-schluessler removed this from the 6.0.0 milestone Apr 3, 2022
This was referenced Apr 3, 2022
Closed
Merged
artonge
artonge reviewed Apr 5, 2022
View reviewed changes
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

Early review :)

src/components/Breadcrumbs/Breadcrumbs.vue Outdated Show resolved Hide resolved
src/components/Breadcrumbs/Breadcrumbs.vue Show resolved Hide resolved
src/components/CheckboxRadioSwitch/CheckboxRadioSwitch.vue Show resolved Hide resolved
src/components/EmojiPicker/EmojiPicker.vue Outdated Show resolved Hide resolved
src/components/Highlight/Highlight.vue Show resolved Hide resolved
src/components/Multiselect/EllipsisedOption.vue Outdated Show resolved Hide resolved
src/components/TimezonePicker/timezoneDataProviderService.js Show resolved Hide resolved
src/functions/usernameToColor/usernameToColor.js Outdated Show resolved Hide resolved
artonge added a commit that referenced this pull request Apr 5, 2022
@artonge
Comply with eslint for event name
b9800f1 
Breaking change.

Follow-up of #2594
@artonge artonge mentioned this pull request Apr 5, 2022
Merged
@raimund-schluessler raimund-schluessler marked this pull request as ready for review April 5, 2022 15:37
@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews and removed 2. developing Work in progress breaking PR that requires a new major version feature: app-sidebar Related to the app-sidebar component feature: emoji picker Related to the emoji picker component labels Apr 5, 2022
@raimund-schluessler raimund-schluessler self-assigned this Apr 6, 2022
@raimund-schluessler raimund-schluessler changed the title Dependencies/eslint config Upgrade @nextcloud/eslint-config to 7.0.2 Apr 6, 2022
@raimund-schluessler raimund-schluessler force-pushed the dependencies/eslint-config branch 3 times, most recently from 9cc04ca to 3a19271 Compare April 6, 2022 11:36
@raimund-schluessler
Copy link
Contributor Author

Updating cypress to latest as well got me one step further. It runs fine now locally. There just seems to be an issue with the cypress GitHub action now running on the wrong node version.

@raimund-schluessler
Copy link
Contributor Author

Updating cypress to latest as well got me one step further. It runs fine now locally. There just seems to be an issue with the cypress GitHub action now running on the wrong node version.

Unfortunately, I have to correct myself. It still fails locally as well 😞

@raimund-schluessler
Copy link
Contributor Author

Good news is, Cypress finally runs through 🎉 🚀
The bad news is, the snapshots are different. I will check if that's really an issue, or if they just need an update now.

@artonge
Copy link
Contributor

artonge commented Apr 7, 2022

Good news is, Cypress finally runs through tada rocket The bad news is, the snapshots are different. I will check if that's really an issue, or if they just need an update now.

Cool ! Do you know what was causing the issue?

@raimund-schluessler
Copy link
Contributor Author

Good news is, Cypress finally runs through tada rocket The bad news is, the snapshots are different. I will check if that's really an issue, or if they just need an update now.

Cool ! Do you know what was causing the issue?

I guess cypress 6.8 (which is quite outdated, current is 9.5) is not compatible with the updated eslint versions. But I don't know for sure. I will split the upgrade to latest cypress into a separate PR, so it's easier to review (I hope that works). Once cypress is updated, we can have a look again at eslint.

@raimund-schluessler raimund-schluessler mentioned this pull request Apr 7, 2022
Merged
@raimund-schluessler
Copy link
Contributor Author

The bump of cypress to 9.5.3 is in #2613. Once that PR is merged, I will rebase this one here #2594.

@raimund-schluessler raimund-schluessler added 2. developing Work in progress and removed 4. to release Ready to be released and/or waiting for tests to finish labels Apr 7, 2022
@raimund-schluessler raimund-schluessler added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 7, 2022
@raimund-schluessler raimund-schluessler marked this pull request as ready for review April 7, 2022 13:07
@raimund-schluessler
Copy link
Contributor Author

@artonge @skjnldsv Checks run through now. Shall we merge?

@raimund-schluessler
Update @nextcloud/eslint-config
ae45fa9 
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Fix lint issues
d8a0820 
Signed-off-by: Raimund Schlüßler <raimund.schluessler@mailbox.org>
@raimund-schluessler
Copy link
Contributor Author

@artonge @skjnldsv Checks run through now. Shall we merge?

I hope you don't mind that I merge this now. But I would like to proceed with other dependency updates and rebasing this everytime another PR got merged is not my favorite thing to do 🙈

@skjnldsv skjnldsv merged commit 782b31d into master Apr 8, 2022
@skjnldsv skjnldsv deleted the dependencies/eslint-config branch April 8, 2022 08:22
@raimund-schluessler raimund-schluessler mentioned this pull request Apr 17, 2022
Closed
raimund-schluessler pushed a commit that referenced this pull request May 31, 2022
@artonge @raimund-schluessler
Comply with eslint for event name
9bf26a7 
Breaking change.

Follow-up of #2594
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@artonge artonge artonge approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@GretaD GretaD Awaiting requested review from GretaD

@juliushaertl juliushaertl Awaiting requested review from juliushaertl

@JuliaKirschenheuter JuliaKirschenheuter Awaiting requested review from JuliaKirschenheuter

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

@ChristophWurst ChristophWurst Awaiting requested review from ChristophWurst

Assignees

@raimund-schluessler raimund-schluessler

Labels
3. to review Waiting for reviews dependencies Pull requests that update a dependency file technical debt
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@raimund-schluessler @skjnldsv @artonge