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

feat: usage of ESLint v8 #274

Merged
merged 3 commits into from
Mar 8, 2023
Merged

feat: usage of ESLint v8 #274

merged 3 commits into from
Mar 8, 2023

Conversation

voxpelli
Copy link
Member

@voxpelli voxpelli commented Jul 18, 2022

Work in progress on upgrading this module to use the new eslint-config-semistandard@17.0.0

See standard/eslint-config-standard#229

Fixes #273

@voxpelli voxpelli self-assigned this Jul 18, 2022
@welcome
Copy link

welcome bot commented Jul 18, 2022

🙌 Thanks for opening this pull request! You're awesome.

Copy link
Member

@theoludwig theoludwig left a comment

Choose a reason for hiding this comment

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

It might be worth adding a new test to ensure we allow top level-await, as we do for standard:
https://github.com/standard/standard/blob/8d832307d9f4e5875c7c32487e596d29db2cf10f/test/validate-config.js#L11-L16

Except that, this looks good to me! 👍
Is there is anything missing, as this is still a draft?

@voxpelli
Copy link
Member Author

@divlo I wanted to look into eg. the external tests + I was considering whether to wait for the old-test PR in standard and copy that one in here as well

@markwylde
Copy link

Any update on this? Can we get this merged?

@nordluf
Copy link

nordluf commented Jan 7, 2023

Since I am not the only one who wants to use this updated version, I have published an npmjs version: https://www.npmjs.com/package/@umpacken/semistandard

If you want to do a seamless update, you can use an npm alias in your package.json like this:

-    "semistandard": "^16.0.1",
+    "semistandard": "npm:@umpacken/semistandard@^16.1.0",

@wesleytodd
Copy link

Hello my fine folks! I just pulled this branch and ran the tests as well as ran it against a project of mine which uses class private fields (the feature I want) and all seems to work well. I am happy to do what it takes to get help get this across the finish line if someone will but help me know what to do.

A lot has changed, but I used to help maintain happiness which was another similar extension of standard, so if this particular fork (which my team is using a bit) needs help I am happy to if I can. Just taking a guess since this PR has been hanging for a while now that that might be the case (not complaining, I know how this stuff goes).

@voxpelli voxpelli removed their assignment Mar 8, 2023
@wesleytodd
Copy link

Hey @divlo, I noticed Pelle remove themselves so I am thinking maybe you are the right person to help. If this project is having resourcing troubles I am happy to help. If nothing else, we could just automate pulling in the latest standard updates and opening PRs like this so all that is necessary is to spot check it and merge to release. Let me know if you or someone else can help me onboard here so I can help.

@theoludwig
Copy link
Member

Hey! @wesleytodd
Thanks for reaching out!

I think the PR is ready to land as is.
@voxpelli Could you explain, what is the state of the PR, please? What can be done?
However I can't release new version on npm anyway.

There is a similar issue of long standing PR (even merged) but not released in standard-engine.

The standard organization suffers from not enough active maintainers with ability to push new releases on npm.
Not sure what to do about it.

@voxpelli
Copy link
Member Author

voxpelli commented Mar 8, 2023

@divlo I sadly can't remember and sadly have no time to spend on Standard. I do agree that there are not enough active maintainers here.

@wesleytodd
Copy link

Could you explain, what is the state of the PR, please? What can be done?

I tested it out and it worked without issue. Are there other checks you would usually do before releasing? I am happy to do them.

However I can't release new version on npm anyway.
The standard organization suffers from not enough active maintainers with ability to push new releases on npm.

Who has publish on these? I wonder if we could add Release Please and then have whoever has the publish can add the tokens to the repos. Who is it that has publish rights on these two packages?


I can't promise much time, but I have been a big fan of this approach to linting for a long time and really appreciate the work y'all have done to keep it going. If the project is suffering I can both keep an eye out for folks looking to contribute and point them here as well as give a little time/effort to helping automate parts like the merge and release flow so it is less hard to maintain the "forks" of standard.

@theoludwig
Copy link
Member

@wesleytodd We can definitely automate the release process. There is a similar issue open in ts-standard: standard/ts-standard#266.

I'm not familiar with Release Please, but I think semantic-release can be a good fit too, similarly to mightyiam/eslint-config-love#840.

Would be greatly appreciated if any one want to open a PR to setup the GitHub Actions to automate that. 👍
Maybe then someone with the rights on both the GitHub repo and npm can then set the tokens, so we can publish automatically. @standard/team

Meanwhile, we will merge this PR, it is ready. 🚀

@theoludwig theoludwig marked this pull request as ready for review March 8, 2023 18:48
@theoludwig theoludwig changed the title Release stable 17.0.0 feat: usage of ESLint v8 Mar 8, 2023
@theoludwig theoludwig merged commit ccec83a into master Mar 8, 2023
@welcome
Copy link

welcome bot commented Mar 8, 2023

🎉 Congrats on getting your first pull request landed!

@theoludwig theoludwig deleted the release-stable-17 branch March 8, 2023 18:50
@wesleytodd
Copy link

wesleytodd commented Mar 8, 2023

I'm not familiar with Release Please, but I think semantic-release can be a good fit too

My main reason for recommending release please is I trust it's author (Benjamin Coe, node.js core contributor and prolific module author) and it is well supported. For a while I was working with Ben and a few others on stuff related to conventional commits and it was while he was working on Release Please.

I am happy to go whichever route folks already working in this project want, just sharing why I like Release Please :)

Would be greatly appreciated if any one want to open a PR to setup the GitHub Actions to automate that.

Yep, that is exactly why I think this kind of automation would be good. The publish token is still restricted, but "publish rights" are given to anyone with write access on the repo.

@wesleytodd
Copy link

wesleytodd commented Mar 8, 2023

A bit of "how the sausage is made" on these:

  1. Release Please uses the new CC parser package which is much better than the older stuff
  2. semantic-release is still on the older stuff

Not really a good reason to make a decision, but we were working on a v2 for Conventional Commits with the intent to better support monorepos and all the other stuff the original didn't really take into account. If folks ever have time to pick that work back up I think it will likely be where new supported features land first.

@wesleytodd
Copy link

@voxpelli I see you have publish rights. Would you mind adding @divlo or passing the responsibility to someone else on the list who has published? I see a few others on there but didn't want to ping them in unless you were unable to help.

@voxpelli
Copy link
Member Author

voxpelli commented Mar 8, 2023

@wesleytodd Sorry, maybe @feross, @LinusU, @Flet or someone else knows

@wesleytodd
Copy link

Thanks! I didn't want to bother you further, just wasn't sure who to ping who had publish rights.

@voxpelli
Copy link
Member Author

Since I did another release in the standard org I threw this in as well, 17.0.0 released

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

Successfully merging this pull request may close these issues.

Parsing error for static class variables
5 participants