ci: auto npm publish script

[import-brain]

fixes #268

[codecov[bot]]

Codecov Report

Merging #757 (b4c47e2) into main (462ee5a) will increase coverage by 0.00%. The diff coverage is n/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:

[Shinigami92]

This looks ridiculously easy :thinking: sus?

[import-brain]

This looks ridiculously easy :thinking: sus?

May or may not have been copy-pasted from GitHub documentation😂

[xDivisionByZerox]

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?

[Shinigami92]

Does the npm publish automatically include a build?

Yes it does, since 6.1.1

https://github.com/faker-js/faker/commit/62dcfc9658e6d3ee23b9e674d10b9a229b026d0e#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R76

But this PR would not change something for that, cause it now would do it for manually publishes too

[xDivisionByZerox]

Ok, I didn't notice that. But tbh I'm not a fan of relying on pre/post scripts for pipelines since:

1. I want to see what a pipeline does by looking at its config file
2. A change in this script can slip in too easy without noticing it directly

Just some points you might want to consider.

[ST-DDT]

@xDivisionByZerox So you would like us to explicitly call build or what?

[xDivisionByZerox]

@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.

[pkuczynski]

I am with @xDivisionByZerox on this...

[ST-DDT]

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).

[xDivisionByZerox]

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.

[prisis]

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 :)

[ST-DDT]

Looks like we won't have a pipeline by Monday.

[prisis]

Nope, sorry... , we can talk about it tomorrow in the call :)

[import-brain]

Superseded by #1096

[ST-DDT]

@import-brain Do you still need the branch? If not, please delete it.

[import-brain]

@import-brain Do you still need the branch? If not, please delete it.

Done