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(cli): add --ignore option #317

Merged
merged 4 commits into from
Nov 23, 2022

Conversation

romkhub
Copy link
Contributor

@romkhub romkhub commented Nov 22, 2022

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[ ] Bug fix
[x] New feature
[ ] Other, please explain:

What changes did you make? (Give an overview)
Added --ignore flag support.
I'd like to run linter separately for unit-tests:

"lint:src": "ts-standard src -p tsconfig.json --ignore \"src/**/*.test.ts\"",
"lint:unit": "ts-standard \"src/**/*.test.ts\" -p tsconfig.test.json",

Which issue (if any) does this pull request address?

Is there anything you'd like reviewers to focus on?
I had to change engines setup in package.json because check:installed-check script fails otherwise:

-    "node": "^12.22.0 || ^14.17.0 || >=16.0.0"
+    "node": "^14.18.0 || >=16.0.0"

this comes from the following dependency-check commit - dependency-check-team/dependency-check@e88ae34

I'd love to see this feature within the current major version, but I doubt this is going to be blessed due to drop of node-12. Is there an option to avoid a major version bump, or should I just go ahead and make it "version": "16.0.0"?

@welcome
Copy link

welcome bot commented Nov 22, 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.

Thanks!
I honestly thought, we already supported the --ignore option. 😄
Overall changes, looks good to me. 👍

About the check:installed-check, I'd rather still support Node.js v12 to avoid the breaking change and in package.json, set "dependency-check": "5.0.0-4",, without the ^.
This will allow us, to release it as standard-engine v15.1.0 and also release it quicker for the different engines like ts-standard. Thoughts @voxpelli?

@voxpelli
Copy link
Member

Should work to just use the ignore flag in installed-check to have it ignore dependency-check, the dev deps are not as crucial and dependency-check will quit silently on unsupported node versions

Copy link
Member

@LinusU LinusU left a comment

Choose a reason for hiding this comment

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

Neat! 👍

@romkhub
Copy link
Contributor Author

romkhub commented Nov 23, 2022

Thank you @voxpelli
done!

@theoludwig theoludwig changed the title Support "--ignore" flag feat(cli): add --ignore option Nov 23, 2022
@theoludwig theoludwig merged commit 21e212c into standard:master Nov 23, 2022
@welcome
Copy link

welcome bot commented Nov 23, 2022

🎉 Congrats on getting your first pull request landed!

@romkhub
Copy link
Contributor Author

romkhub commented Nov 28, 2022

Good day
Thank you a lot for accepting this PR
Could I kindly ask anyone to release the 15.1.0 version?

@romkhub
Copy link
Contributor Author

romkhub commented Dec 14, 2022

@voxpelli @divlo @LinusU
sorry for bothering, could you please help releasing this?

@voxpelli
Copy link
Member

I'll try to get to it next week, I'm crunching g towards a Christmas deadline right now, if it hasn't been released by next week, then ping me here again

(I want GitHub to add a snooze feature 😝)

@theoludwig
Copy link
Member

friendly ping @voxpelli

@romkhub
Copy link
Contributor Author

romkhub commented Feb 14, 2023

friendly ping @voxpelli ❤️

@voxpelli
Copy link
Member

Released in 15.1.0, thanks 🙏

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

Successfully merging this pull request may close these issues.

4 participants