Closed import-brain closed 1 year ago
Merging #757 (b4c47e2) into main (462ee5a) will increase coverage by
0.00%
. The diff coverage isn/a
.
@@ Coverage Diff @@
## main #757 +/- ##
=======================================
Coverage 99.62% 99.62%
=======================================
Files 2156 2156
Lines 237025 237025
Branches 1006 1008 +2
=======================================
+ Hits 236126 236136 +10
+ Misses 878 868 -10
Partials 21 21
Impacted Files | Coverage Δ | |
---|---|---|
src/modules/internet/user-agent.ts | 84.39% <0.00%> (+2.64%) |
:arrow_up: |
This looks ridiculously easy :thinking: sus?
This looks ridiculously easy :thinking: sus?
May or may not have been copy-pasted from GitHub documentation😂
Isn't the point of this pipeline to ensure that things like v6.1.0 don't happen again? Well, the pipeline doesn't build the newest state of the library. In fact, it doesn't build the project at all. Or did I miss something? Does the npm publish
automatically include a build?
Does the
npm publish
automatically include a build?
Yes it does, since 6.1.1
But this PR would not change something for that, cause it now would do it for manually publishes too
Ok, I didn't notice that. But tbh I'm not a fan of relying on pre/post scripts for pipelines since:
Just some points you might want to consider.
@xDivisionByZerox So you would like us to explicitly call build
or what?
@xDivisionByZerox So you would like us to explicitly call build or what?
I would do so for my pipelines, yes. But I don't think I'm in the position to demand anything.
I guess it would be helpful enough, for anyone that might have a look at the config in the future, to add a comment over the npm publish
, that there is a pre-publish script in place.
I am with @xDivisionByZerox on this...
I am with @xDivisionByZerox on this...
Would something like this suffice for you?
# Relies on the prepublishOnly hook to build the project
- run: npm publish
Unfortunately it isn't possible to add comments to the package.json to add a hint there too. IMO the info in the pipeline is not really helpful, because it only state what it depends on (and that it's nothing extraordinary for an Node dev), but the other side contains no reference to dependent steps (and AFAICT these vary greatly between projects).
Would something like this suffice for you?
# Relies on the prepublishOnly hook to build the project - run: npm publish
Unfortunately it isn't possible to add comments to the package.json to add a hint there too. IMO the info in the pipeline is not really helpful, because it only state what it depends on (and that it's nothing extraordinary for an Node dev), but the other side contains no reference to dependent steps (and AFAICT these vary greatly between projects).
I mean if you state it that way, why not explicitly call the necessary build steps? Yes, it's a bit redundant and takes some more time to complete, on the other side this is run like once a week, so it shouldn't matter.
Even going a step further: Why not remove the prepublish
script from the package? When a release pipeline is in place NOONE should make a manual release EVER.
I would go more in this direction https://www.npmjs.com/package/@changesets/cli, check https://github.com/Thinkmill/manypkg as example how it works, it creates a .changeset folder with all the changes than can be than released
In the end we need to discuss how the workflow should be, and what we want :)
Looks like we won't have a pipeline by Monday.
Nope, sorry... , we can talk about it tomorrow in the call :)
Superseded by #1096
@import-brain Do you still need the branch? If not, please delete it.
@import-brain Do you still need the branch? If not, please delete it.
Done
fixes #268