-
Notifications
You must be signed in to change notification settings - Fork 104
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
Using the git urls with specific branch or commit version in package.json does not work #102
Comments
What do you propose? We should not rebuild the files for every commit. The git repo will explode. Who does this anyway? That's what NPM is for. If you are fiddling with git versions in your package.json, you should be aware of this problem. Others do it just the same, don't they? |
To me it is not sure what to do and howthe situation can be improved. I wrote the report to show that two people here ran into the problem. Now it is documented for discussions and others to find and hopefully avoid the problem. From reading the npm documentation at https://docs.npmjs.com/files/package.json#git-urls-as-dependencies the expectation is raised that using a github url with branch or commit detail will lead So far I cannot say if changing the install or build script in svg2pdf.js' package.json will lead to this expected result. I agree with you that rebuilding the files in dist for each commit maybe a bit much and I don't know yet how others do it. |
One solution we are thinking of is to use our clone on this platform to actually rebuild the dist files for each commit when we need it. This probably can be automated somehow. However I'd prefer a better solution. |
I agree with @yGuy: it makes no sense to commit the dist files in every commit. And depending on the master branch is especially risky, since it may change any time and may introduce new bugs that are fixed later. The only stable versions are those marked with version tags. And on these commits the dist files are up to date. If you need to depend on a specific commit then the best solution is to fork the repo and depend on it. You can switch back to an npm dependency once we released the changesets you require. |
Of course, if you depend on a specific commit, you can just checkout that commit and run the build manually in your deployment chain. Other than that: the fact that with each tagged release the dist is or should be up-to-date really makes this a very minor issue, IMHO. |
Thanks for your comments, I agree that the issue is minor, once known. It can lead to a bad experience if not known (as we were debugging in the wrong direction). Searching the web shows that there is no good practice for how to update See that the discussion in https://stackoverflow.com/questions/17509669/how-to-install-an-npm-package-from-github-directly shows that some people expect a package to installable from git and specify a script for doing so. However https://docs.npmjs.com/misc/scripts currently only recommends using |
Hi, I am the second developer (co-worker of Bernhard) who ran besides him into said problem. @HackbrettXXX you are totally correct in stating that relying on ATM I see this mostly as lessons learned in the npm-universe. Looking at your release history I see several releases this year which is from a "consumer-perspective" nice. Since @yGuy asked about possible proposals. I think, a proposal might be: If the release gap is "longer" ( I am aware of the subjective nature here) having something like rc-tags as in between releases with a proper dist would be nice to have. OTOH I am fully aware that this depends on your manpower, workload etc. and doing releases biyearly is totally fine. But I was asked for proposals (="wishes") ;) And I am more than grateful, that you guys make this happen 👍 From my POV this issue could be closed - and as @bernhardreiter said is only of historical value in case someone besides us falls into the for him unexpected pit. |
My suggestions for a solution are:
Additionally helpful would have been a version string in the minified result that indicates the development nature, if the file was build from inbetween commits. |
I vote for the "improve the documentation" option, but this really is a very minor issue. |
@yGuy thanks for your comment and maintaining svg2pdf as Free Software! My suggestion for a documentation hint:
To me it is more than a minor issue, because even if only a few people fall into this trap, the trap is several work-hours deep. And it would be a bad experience with svg2pdf nontheless it was caused by the way the npm/yarn world handles this. |
yarn add https://github.com/yWorks/svg2pdf.js.git#master
will checkout the latest version from git, but will not update the files which are in
dist
.Those are old, so while believing you have a never version, you haven't.
The text was updated successfully, but these errors were encountered: