-
Notifications
You must be signed in to change notification settings - Fork 35
Support globs for src and ignoring paths #165
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
base: master
Are you sure you want to change the base?
Conversation
The dependency update is needed in order to use glob and similar packages
Fixtures and test included
| "jest": "^30.2.0", | ||
| "lint-staged": "^5.0.0", | ||
| "prettier": "^1.19.1", | ||
| "ts-jest": "^26.3.0", | ||
| "ts-jest": "^29.4.6", | ||
| "ts-node": "3.3.0", | ||
| "typescript": "^3.9.7" | ||
| "typescript": "^4.3" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These may be good changes, but they are strictly unrelated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the description, the glob package required a higher version of typescript, thus creating a chain reaction that required updating jest too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ugh... I would personally prefer dependency bumps to be a separate PR, but then again I'm not the owner of this project, maybe they think it's OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see the value of splitting it into multiple PRs but would appreciate the input of someone with merge and publish privileges.
If you have the rights @dimaqq, I will get right into it 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OMG the diff size!
| "msgid "" | ||
| msgstr "" | ||
| "Content-Type: text/plain; charset=utf-8\\n" | ||
| "Plural-Forms: nplurals=2; plural=(n!=1);\\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are these being changed?
(here, across this file and many other files).
It feels like the change is semantic, the (escaped) backslashes are removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I can tell, Jest has changed how snapshots are formatted from v24 to v30.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's Jest 29, where escapeString: false is the new default (?)
I would propose that the jest bump is a separate PR, without prod code changes.
Yes, that's a bit of extra work, but hopefully it's mechanical work and easy to review.
Co-authored-by: Dima Tisnek <[email protected]>
Co-authored-by: Dima Tisnek <[email protected]>
This PR is a proposal for adding glob and ignore capabilities to the ttag-cli while keeping backwards compatibility (#78).
Feedback is welcome! 🙂
The feature has been built around the following goals:
Change summary:
globpackage requires a higher version oftypescript, thus creating a chain reaction that required updatingjesttoo).jestversion bump.src/index.tsto inject theignoreargument into thecommands/*.src/libs/utils.ts > resolvePaths. No changes were made to the existing file-walking logic.