Skip to content
This repository has been archived by the owner on Nov 10, 2020. It is now read-only.

Pin package dependencies to specific versions #1570

Closed
wants to merge 1 commit into from

Conversation

dmsnell
Copy link
Contributor

@dmsnell dmsnell commented Jul 5, 2016

In #1565 I ran into an issue where changes were brought in to the
minified javascript bundle that were unrelated to the changes made in
the PR. This was due to NPM packages which were pulled in under
the compatability version spec pkg: ^1.2.3 but which differed from the
versions used to build the current release in dev.

This PR only pins the versions of the three libraries that interfered
with #1563, but it might be worthwhile to ammend it and pin all of the
dependency versions to ensure that all development environments are
building with the same code.

cc: @gemfarmer

Cheers!

P.S. queue-async has been deprecated and renamed to d3-queue

@dmsnell
Copy link
Contributor Author

dmsnell commented Jul 8, 2016

For what it's worth, I encourage removing all of the rest of those ^s, but didn't want to make that jump here in this PR without any feedback from you.

When working like this, I have found ncu a handy tool for managing and updating packages. Version updates then go into their own PRs and get the full review that normal code changes get.

@gemfarmer
Copy link
Contributor

Hey @dmsnell thanks for the second PR and I apologize for the delayed response.

I think this is a good suggestion, but I will want to wait to discuss this with @shawnbot before merging.

Also to note, most of the recent dev work has happened on the state-pages branch. We have updated queue-async to d3-queue on that branch and it should be merged to dev some time this month.

@dmsnell
Copy link
Contributor Author

dmsnell commented Jul 11, 2016

Thanks @gemfarmer - would it be helpful then to rebase against state-pages? Is there a central master branch? Pardon my confusion - I may just not used to working with the process on this project.

@shawnbot
Copy link
Contributor

No problem, and thanks for taking a look at this @dmsnell! We've been off on the state-pages branch for a while now because it involves some dramatic changes in the site structure that we hope to merge soon. I think it'd be best to wait until after that work is merged to dev, then we can look into merging this.

Fair warning: There will most likely be merge conflicts in package.json, since we've added other dependencies.

@dmsnell
Copy link
Contributor Author

dmsnell commented Jul 12, 2016

Well @shawnbot, I guess I'll just have to risk dealing with a merge conflict on a +3/-3 PR 😉

git checkout dev
git branch -D update/pin-dependencies
git checkout -b update/pin-dependencies
sed 's/\^//' package.json

😄

In DOI-ONRR#1565 I ran into an issue where changes were brought in to the
minified javascript bundle that were unrelated to the changes made in
the PR. This was due to NPM packages which were pulled in under
the compatability version spec `pkg: ^1.2.3` but which differed from the
versions used to build the current release in `dev`.

This PR only pins the versions of the three libraries that interfered
with DOI-ONRR#1563, but it might be worthwhile to ammend it and pin all of the
dependency versions to ensure that all development environments are
building with the same code.

Cheers!
@dmsnell dmsnell force-pushed the update/pin-dependencies branch from 4831880 to 5c25537 Compare July 19, 2016 04:09
@shawnbot
Copy link
Contributor

Sorry @dmsnell, I just realized what I meant by merge conflicts: As it stands, we'll need to manually apply this after we've merged what we're calling the Great State Page Migration in #1355 (the state-pages branch). You can try rebasing against that branch in the meantime... if you so dare! 👻

@dmsnell
Copy link
Contributor Author

dmsnell commented Jul 22, 2016

Thanks @shawnbot. Likewise, what I meant is that there is nothing manual that needs to be done. When/if you want to do this, it's literally as simple as removing the carets from the versions, or adding version numbers. Basically, I am fine closing this PR as there would be little difference in redoing the work here and creating a new one.

Great State Page Migration

reminds me of the Great Vowel Migration

@dmsnell
Copy link
Contributor Author

dmsnell commented Oct 5, 2016

I'm closing this PR as it is now irrelevant. Pinning is still good but this is out of date.

@dmsnell dmsnell closed this Oct 5, 2016
@dmsnell dmsnell deleted the update/pin-dependencies branch October 5, 2016 22:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants