This repository has been archived by the owner on Jun 4, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 49
Remove events #89
Merged
Merged
Remove events #89
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
37febab
lint generate-components
alexcjohnson 173510e
npm test shortcut
alexcjohnson 2f873da
ensure element list is unique
alexcjohnson 887846f
remove events from generate-components script
alexcjohnson 599d98c
rebuild
alexcjohnson a2e145e
update changelog
alexcjohnson bdfbb5c
add PR number to changelog for :hocho: events
alexcjohnson eb3c4de
lint scripts dir
alexcjohnson 82695c1
simplify ci command & include lint
alexcjohnson 52b049d
set +e along with lint and test errors
alexcjohnson 3ad1dd3
fix lint error only - still has test error
alexcjohnson 2930061
remove set +e
alexcjohnson 15d2891
set +e and +o pipefail - still failing test only, not lint
alexcjohnson e42be85
new approach: npm-run-all -> run-s
alexcjohnson f28aede
remove fake test error
alexcjohnson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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'd do the opposite and have linting run as early as possible (before
npm run clean
), to prevent executing a long-ish build and tests for nothing.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.
Or in this case, I guess the .js files are also generated -- so after the generation.
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 could move it, but I wouldn't want linter errors to prevent running the rest of the tests. The tests aren't that expensive, seems to me it's worth knowing about all failures together. Personally linter errors are about the last thing I deal with - in the unusual case that they're not caught automatically by my editor, forcing me to fix them right away.
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.
Fair enough for the ordering. I had scenarios like the table where running the tests takes up to 15 minutes :)
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.
Actually, I don't think you'll get all the errors as-is. If tests fail, the job will terminate before linting happens. You would need a job for tests and one for linting to always get both sources of failure.
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.
Good point. Looks like we need a
set +e
perhaps? Personally I'd rather get all the errors even in cases where the tests take 15 minutes...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.
Nice! That should also work.
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.
OK, my bash-fu is not the best, but I couldn't find a straight shell solution that worked the way I wanted... everything I tried either stopped on first failure or ignored errors if something later passed.
npm-run-all
and itsrun-s
command (with the-c
option) behaves the way I wanted, running everything no matter what, and reporting an error from any part.