ci: Don't --focus install JS deps#11267
Merged
Merged
Conversation
johncowen
added
theme/ui
johncowen
added
the
pr/no-changelog
Collaborator
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/468276. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In #9049 we tried out storybook for cataloging and documenting our component files. One of the problems with that was that storybook has oodles of dependencies that we didn't really want to have folks have to download/install if they only wanted to build the production application.
The JS community/ecosystem gets around this problem by having separate dependencies for developer tooling in the
devDependenciesproperty of thepackage.jsonfile. Conversely, conventional ember development dictates you should also use thisdevDependencyproperty for your shipped projectdependenciesalso. Back in #5992 we tried a different more JS traditional convention to work around this, but that was never approved.When started depending on storybook we tried using
yarn installwith the--focusflag to as another attempt to prevent folks from having to install the 'kitchen sink' when all they wanted was a production UI. Unfortunately this didn't quite work out either, but the--focusflag in our build script has stuck around for one reason or another.Since then, we've uninstalled storybook and used the marvellous docfy to catalog and document our components, plus other things. An additional benefit of this has been far less dependencies for our documentation, so the 'please don't download so much stuff' problem has pretty much just gone away.
Unfortunately, the left over yarn
--focusflag has caused a few issues in CI, as CI generally installs from the root of the workspace whereas engineers generally install from theconsul-uiroot.This PR brings everything together and reverts everything to just using
make(with no--focus), which will do the right thing where-ever you are ( 🤞 ).