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.

Sign up for GitHub

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

Jump to bottom

Set package.json version to 0.0.0 #2692

Merged
merged 1 commit into from
Jul 14, 2017
Merged

Set package.json version to 0.0.0 #2692

merged 1 commit into from
Jul 14, 2017

Conversation

rndstr
Copy link
Contributor

@rndstr rndstr commented Jul 6, 2017

The package version is irrelevant for the build process
and is not read anywhere.

The package is not published and causes confusion if the
bump is forgotten.

@rndstr rndstr requested review from jpellizzari and davkal July 6, 2017 20:35
@jpellizzari
Copy link
Contributor

jpellizzari commented Jul 6, 2017 •
edited
Loading

We do install Scope as an npm module in service-ui. This change shouldn't affect that since we use the commit hash of a .tgz on S3, but a test wouldn't hurt.

@rndstr
Set package.json version to 0.0.0
0d2a1e3 
The package version is irrelevant for the build process
and is not read anywhere.

The package is not published and causes confusion if the
bump is forgotten.
@rndstr rndstr force-pushed the nodejs-package-version-reset branch from 43410fa to 0d2a1e3 Compare July 10, 2017 11:36
jpellizzari
jpellizzari approved these changes Jul 13, 2017
View reviewed changes
Copy link
Contributor

@jpellizzari jpellizzari left a comment

Choose a reason for hiding this comment

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

LGTM, but there is a small chance that this will confuse yarn or npm when service-ui installs Scope. Shouldn't be an issue, since we specify a commit hash.

@rndstr
Copy link
Contributor Author

rndstr commented Jul 14, 2017

Thanks Jordan (it did work when I tried it locally)

@rndstr rndstr merged commit a3cd457 into master Jul 14, 2017
@rndstr rndstr deleted the nodejs-package-version-reset branch July 14, 2017 08:24
@fbarl
Copy link
Contributor

fbarl commented Jul 27, 2017

I'm afraid this PR have screwed something with yarn.lock in Service UI. It was only updated in https://github.com/weaveworks/service-ui/commit/89dd0d3ee08d6c1991262b5178fe2f599a35d062, but that commit didn't really update the Scope version in the UI (even though the right version was shown in the footer). Still not sure it has to do with this PR though, but maybe we should see if reverting fixes it. :/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jpellizzari jpellizzari jpellizzari approved these changes

@davkal davkal Awaiting requested review from davkal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rndstr @jpellizzari @fbarl