Skip to content

feat(eslint-plugin): extend eslint semi rule #411

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

Closed

Conversation

aprilrd
Copy link
Contributor

@aprilrd aprilrd commented Apr 6, 2019

Fixed #366, #123 and #409.

I plan on fixing #123 and #409 in this pr as well.

@aprilrd aprilrd force-pushed the type-aliase-semicolon branch 2 times, most recently from 68d13ad to 646d44f Compare April 6, 2019 12:15
@aprilrd aprilrd changed the title feat(semi): extend eslint semi rule feat(eslint-plugin): extend eslint semi rule Apr 6, 2019
@aprilrd aprilrd marked this pull request as ready for review April 6, 2019 14:05
@aprilrd aprilrd force-pushed the type-aliase-semicolon branch from 646d44f to 9d775d5 Compare April 6, 2019 14:08
@codecov
Copy link

codecov bot commented Apr 7, 2019

Codecov Report

Merging #411 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #411      +/-   ##
==========================================
+ Coverage   97.14%   97.15%   +<.01%     
==========================================
  Files          74       75       +1     
  Lines        2875     2885      +10     
  Branches      473      473              
==========================================
+ Hits         2793     2803      +10     
  Misses         49       49              
  Partials       33       33
Impacted Files Coverage Δ
packages/eslint-plugin/src/rules/semi.ts 100% <100%> (ø)

@bradzacher bradzacher added the enhancement: new plugin rule New rule request for eslint-plugin label Apr 11, 2019
Copy link
Member

@bradzacher bradzacher left a comment

Choose a reason for hiding this comment

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

Two things, otherwise looks good to me:

@aprilrd aprilrd force-pushed the type-aliase-semicolon branch from a1976af to 8c5c1c9 Compare April 12, 2019 14:29
@bradzacher bradzacher added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Apr 19, 2019
@JamesHenry
Copy link
Member

JamesHenry commented Apr 24, 2019

Wait, @bradzacher what happened here?

I went down PRs in reverse chronological order, so it is also partly my fault for not checking earlier contributions, but I have just merged a PR from you for this exact rule, despite you having approved an already open PR?

@JamesHenry
Copy link
Member

I'm sorry for the confusion here @aprilrd I had no intention of missing our your contribution, thanks so much for taking the time to do it

@aprilrd
Copy link
Contributor Author

aprilrd commented Apr 24, 2019

It is a bummer. If you would be so kind, I would like to understand the issues with this PR so that I can do better next time.

@bradzacher
Copy link
Member

Ahh crap, I'm so sorry about this.
You did nothing wrong @aprilrd, this was 100% my fault; I wasn't paying enough attention when going through the issue log.

I was going through adding fixes for issues tagged as bug, noticed 3 issues for semi, and prioritised it; completely forgetting that there was this open PR that I'd already reviewed a few weeks ago.

If you're willing - there's a few places that your PR can still contribute to this - specifically:

  • your much, much more comprehensive tests,
  • your update to packages/eslint-plugin/ROADMAP.md

Again, I'm sorry that I slipped up here.

@aprilrd
Copy link
Contributor Author

aprilrd commented Apr 25, 2019

I understand. I will make a separate pr with the readme and the test.

@aprilrd aprilrd closed this Apr 25, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge enhancement: new plugin rule New rule request for eslint-plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[semi] Semicolons aren't checked for type aliases
3 participants