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

fix set label to translation pages #2

Merged
merged 12 commits into from
Nov 24, 2021
Merged

fix set label to translation pages #2

merged 12 commits into from
Nov 24, 2021

Conversation

navarroaxel
Copy link
Contributor

@navarroaxel navarroaxel commented Nov 21, 2021

Simplify regex for test directories and files.
Extract testable getFileLabel function.
Simplify loop over changed files.
Use Array instead of Set.
Remove waiting label only.
Add community label for MAINTAINERS.md file.
Add unit tests with jest.
Add husky for run unit tests before push.
Add engine to package.json to know the required runtime easier.

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Show resolved Hide resolved
src/labeler.spec.ts Outdated Show resolved Hide resolved
@@ -0,0 +1,130 @@
const {getFileLabel, fileStatus, labelType} = require('./labeler')
Copy link
Contributor

Choose a reason for hiding this comment

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

Both tldr-node-client and tldr-lint have their tests in their own directory. I would suggest doing the same here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I create a folder outside `src, I get this error:

$ ncc build src/index.ts --out dist
ncc: Version 0.32.0
ncc: Compiling file index.js into CJS
ncc: Using typescript@4.5.2 (local user-provided)
Error: [tsl] ERROR
TS6059: File '/home/user/src/tldr-labeler-action/test/labeler.spec.ts' is not under 'rootDir' '/home/user/src/tldr-labeler-action/src'. 'rootDir' is expected to contain all source files.
The file is in the program because:
Root file specified for compilation
at /home/user/src/tldr-labeler-action/node_modules/@vercel/ncc/dist/ncc/index.js.cache.js:37:1896789
at /home/user/src/tldr-labeler-action/node_modules/@vercel/ncc/dist/ncc/index.js.cache.js:37:353322
at _done (eval at create (/home/user/src/tldr-labeler-action/node_modules/@vercel/ncc/dist/ncc/index.js.cache.js:20:117937), :7:1)
at eval (eval at create (/home/user/src/tldr-labeler-action/node_modules/@vercel/ncc/dist/ncc/index.js.cache.js:20:117937), :32:22)

Copy link
Contributor

Choose a reason for hiding this comment

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

You will probably need to explicitly say what files to include in the tsconfig.json file. This error also demonstrates why we want this file outside of the ncc build, in that we do not want to risk bundling our test sources with our source code for distribution.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test file is not included in the bundle.

src/labeler.spec.ts Outdated Show resolved Hide resolved
src/labeler.spec.ts Outdated Show resolved Hide resolved
src/labeler.spec.ts Outdated Show resolved Hide resolved
src/labeler.spec.ts Outdated Show resolved Hide resolved
src/labeler.spec.ts Outdated Show resolved Hide resolved
@@ -0,0 +1 @@
12.22
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure we can't use anything more modern here? The latest LTS version is 16.x.....

Copy link
Contributor Author

@navarroaxel navarroaxel Nov 23, 2021

Choose a reason for hiding this comment

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

Nope, the actions run in node12 only.

Copy link
Contributor

Choose a reason for hiding this comment

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

Though this only applies for local environments, and does not change anything about the action environment itself, which is controlled in the action.yml file.

However, again, making it so that only people with node12 (or want to switch to it) seems rather heavy handed to ensure node12 compliance...

dist/index.js Show resolved Hide resolved
src/labeler.ts Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
Co-authored-by: Matthew Peveler <matt.peveler@gmail.com>
Co-authored-by: Starbeamrainbowlabs <sbrl@starbeamrainbowlabs.com>
@navarroaxel navarroaxel merged commit f69e2aa into main Nov 24, 2021
@navarroaxel navarroaxel deleted the fix/mark-labels branch November 24, 2021 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants