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.

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

Upgrade eslint to v8.x #252

Merged
merged 2 commits into from
Jul 13, 2022
Merged

Conversation

BilalQamar95
Copy link
Contributor

Ticket
42: Upgrade eslint to v8.x

Description:

  • Upgraded eslint to v8.18 along with @edx/eslint-config
  • Resolved linting issues post upgrade.

@BilalQamar95 BilalQamar95 self-assigned this Jun 24, 2022
@BilalQamar95 BilalQamar95 requested a review from a team June 24, 2022 08:20
@davidjoy
Copy link
Contributor

So one thing we're probably going to want to do is to understand what, if any, rules we're about to break in the applications and packages that depend on frontend-build when we merge this. i.e., we've upgraded eslint through multiple major version upgrades, what changed? Will all our apps suddenly start having a bunch of linting errors? If rules have changed, I think we need to make this a major version bump of frontend-build. @adamstankiewicz and @arbrandes, what do you think?

@davidjoy
Copy link
Contributor

It's a bit nebulous with a package like frontend-build, but I think I might consider this a feat: PR, possibly with a breaking change as described in my previous comment.

@BilalQamar95
Copy link
Contributor Author

BilalQamar95 commented Jul 6, 2022

So I do see some of our apps, depending on frontend-build, to have linting issues with this update.

Details regarding breaking changes

Copy link
Contributor

@davidjoy davidjoy left a comment

Choose a reason for hiding this comment

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

I say we consider this a breaking change in frontend-build if it'll result in linting issues. That way folks can be intentional about upgrading to it, rather than having it blow up their build.

So FYI, to do that with semantic-release, the commit message needs to include BREAKING CHANGE: and we should describe the breaking changes in as much detail as we can. That'll cause semantic-release to bump frontend-build's version number by a major increment.

If we squash commits and specify a new commit message with feat: in the subject and BREAKING CHANGE: in the body, that'll be sufficient. I'm going to approve with the expectation that we'll just do that.

@BilalQamar95 BilalQamar95 merged commit 0c03c44 into master Jul 13, 2022
@BilalQamar95 BilalQamar95 deleted the bilalqamar95/fed-bom-eslint-upgrade branch July 13, 2022 08:42
@edx-semantic-release
Copy link

🎉 This PR is included in version 12.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants