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.

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

binaries: remove generated node_modules binaries and compiled .js files #42

Closed
wants to merge 2 commits into from

Conversation

ljmf00
Copy link
Contributor

@ljmf00 ljmf00 commented Feb 8, 2021

Signed-off-by: Luís Ferreira [email protected]

@ljmf00 ljmf00 marked this pull request as draft February 8, 2021 16:24
@ljmf00 ljmf00 marked this pull request as ready for review February 8, 2021 16:39
@ljmf00
Copy link
Contributor Author

ljmf00 commented Feb 8, 2021

Closing this. Javascript and this Github action integration is just bad :(

@ljmf00 ljmf00 closed this Feb 8, 2021
@WebFreak001
Copy link
Member

the node_modules is required by github actions, it doesn't do an install

@ljmf00
Copy link
Contributor Author

ljmf00 commented Feb 8, 2021

the node_modules is required by GitHub actions, it doesn't do an install

Yes. A better way would be to bundle only the required stuff from the dependencies in a file and have a developing branch and then deploy the generated files the actual action to the release branches (v1, v2, ...)

@mihails-strasuns
Copy link
Collaborator

Each PR branch is effectively a separate devel branch, so this is effectively how it works already. We have agreed to switch node_modules to a bundled version though, so it is only a matter of someone doing the implementation (would be cool to do it automatically on a PR merge).

@ljmf00
Copy link
Contributor Author

ljmf00 commented Feb 12, 2021

Each PR branch is effectively a separate devel branch, so this is effectively how it works already. We have agreed to switch node_modules to a bundled version though, so it is only a matter of someone doing the implementation (would be cool to do it automatically on a PR merge).

What I was trying to say is having a development branch in the upstream instead of pushing directly to version branches or changing tags, this is generally a bad practice because changing a version, for whatever reasons is bad. When a user relies on a version, that version shouldn't change, and therefore, the upstream branch related to that version shouldn't change too.

@mihails-strasuns
Copy link
Collaborator

But a version is a tag. If you depend on untagged version branch in your yml, you are explicitly opting in for the latest version of that branch. I could understand the concern if there was any additional stabilization stage needed, but for setup-dlang anything that is good enough to pass CI in PR, is good enough to be immediately released (and there is no extra verification involved).

@PetarKirov
Copy link
Member

@ljmf00 btw I fixed this issue in #43

@ljmf00
Copy link
Contributor Author

ljmf00 commented Feb 17, 2021

@ljmf00 btw I fixed this issue in #43

Thanks 👍

@ljmf00
Copy link
Contributor Author

ljmf00 commented Feb 17, 2021

But a version is a tag. If you depend on untagged version branch in your yml, you are explicitly opting in for the latest version of that branch. I could understand the concern if there was any additional stabilization stage needed, but for setup-dlang anything that is good enough to pass CI in PR, is good enough to be immediately released (and there is no extra verification involved).

Oh, ok screw that part then. I completely missed the fact that this repo is being tagged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants