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

Feat/add pkgjs workflow #167

Merged
merged 11 commits into from
Feb 24, 2022
Merged

Feat/add pkgjs workflow #167

merged 11 commits into from
Feb 24, 2022

Conversation

radomird
Copy link
Contributor

@radomird radomird commented Dec 9, 2021

I've updated the ci workflow to use https://github.com/pkgjs/action.
Few notes:
- the action is not released yet and thus it's using the @main version
- the "link" to the action is pkgjs/action/.github/workflows/node-test.yaml@main which is a bit unusual (this one directly references the yaml file) compared to other actions for example: fastify/github-action-merge-dependabot@v2.7.1

Other than these two notes I think it's working good.
Update: As @simoneb pointed out, this is a reusable workflow and the reference is correct (https://docs.github.com/en/actions/learn-github-actions/reusing-workflows#calling-a-reusable-workflow)

Closes #204

@radomird radomird requested review from simoneb and Eomm December 9, 2021 10:57
@radomird
Copy link
Contributor Author

radomird commented Dec 9, 2021

Hi @dominykas I've added your reusable workflow to this repo. It works fine.

package.json Show resolved Hide resolved
@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

the "link" to the action is pkgjs/action/.github/workflows/node-test.yaml@main which is a bit unusual

that's because this is not an action, it's a reusable workflow https://docs.github.com/en/actions/learn-github-actions/reusing-workflows

@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

please check out the source code of the reusable workflow to see why it would not work

@radomird
Copy link
Contributor Author

radomird commented Dec 9, 2021

that's because this is not an action, it's a reusable workflow https://docs.github.com/en/actions/learn-github-actions/reusing-workflows

Updated the PR description.

please check out the source code of the reusable workflow to see why it would not work

What do you mean? What wouldn't work?

package.json Outdated Show resolved Hide resolved
@radomird radomird requested review from simoneb and removed request for Eomm December 9, 2021 11:39
Copy link
Member

@simoneb simoneb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I like the idea but there's one thing I'm still not sure about. How do we version this? Do we always have to reference the main branch to use this reusable workflow? Is there a way to use it as an action instead?

@radomird
Copy link
Contributor Author

radomird commented Dec 9, 2021

Overall I like the idea but there's one thing I'm still not sure about. How do we version this? Do we always have to reference the main branch to use this reusable workflow? Is there a way to use it as an action instead?

I think reusable workflows can be versioned the same way as actions, either by tag or addressing specific branch, according to this: https://docs.github.com/en/actions/learn-github-actions/reusing-workflows#reusable-workflows-and-workflow-templates

@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

Overall I like the idea but there's one thing I'm still not sure about. How do we version this? Do we always have to reference the main branch to use this reusable workflow? Is there a way to use it as an action instead?

I think reusable workflows can be versioned the same way as actions, either by tag or addressing specific branch, according to this: https://docs.github.com/en/actions/learn-github-actions/reusing-workflows#reusable-workflows-and-workflow-templates

Yes we would have to, we can't reference a branch name for various reasons. Can you try to figure out if dependabot works for reusable workflows too? e.g. a new tag is created, does dependabot create a PR to update it in the way it does for github actions?

@dominykas
Copy link
Member

This workflow will be versioned - I'd like the API to settle down a little bit before I do that, though, still collecting feedback. I hope to do that early next year - I'll keep an eye on this repo as well and will open a PR once v1 is ready.

I'd also like to add some basic tooling to do the versioning (i.e. I need something that will add a v1, v1.1 when v1.1.1 is added).

@dominykas
Copy link
Member

Can you try to figure out if dependabot works for reusable workflows too?

If it doesn't, there's always renovate (it supports regex based managers, if all else fails).

@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

This workflow will be versioned - I'd like the API to settle down a little bit before I do that, though, still collecting feedback. I hope to do that early next year - I'll keep an eye on this repo as well and will open a PR once v1 is ready.

I'd also like to add some basic tooling to do the versioning (i.e. I need something that will add a v1, v1.1 when v1.1.1 is added).

We have that feature (creating the various tags) in https://github.com/nearform/optic-release-automation-action

@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

Can you try to figure out if dependabot works for reusable workflows too?

If it doesn't, there's always renovate (it supports regex based managers, if all else fails).

we've decommissioned renovate everywhere

@simoneb
Copy link
Member

simoneb commented Dec 9, 2021

@radomird let's keep this PR here. I like the net result but let's wait until the tool reaches its first release. Note that we will need to change the required checks to reflect the checks run by the tool

@radomird
Copy link
Contributor Author

radomird commented Dec 9, 2021

@radomird let's keep this PR here. I like the net result but let's wait until the tool reaches its first release. Note that we will need to change the required checks to reflect the checks run by the tool

Sounds good. I'll move it to "On hold"

@simoneb simoneb marked this pull request as draft December 9, 2021 12:13
@dominykas
Copy link
Member

let's wait until the tool reaches its first release

FWIW, the stuff that you're using is unlikely to change at v1, unless it's specifically the @main you're concerned about for other reasons.

@radomird radomird self-assigned this Dec 15, 2021
@dominykas
Copy link
Member

dominykas commented Jan 18, 2022

@simoneb would you be happy enough to start using it if I tagged a @v0.1 there (rather than just @main)?

There's some hesitance about tagging a @v1... (not that I agree with that hesitance)

@simoneb
Copy link
Member

simoneb commented Jan 19, 2022

@simoneb would you be happy enough to start using it if I tagged a @v0.1 there (rather than just @main)?

There's some hesitance about tagging a @v1... (not that I agree with that hesitance)

Yes. I'm honestly not completely sold on the idea of using this approach thoroughly across repos, but happy to try it out and see how it works out.

Just to mention one thing, the required checks in the repo need to be aligned, but I'm sure there are more gotchas we'll need to look out for.

@dominykas
Copy link
Member

dominykas commented Jan 31, 2022

not completely sold on the idea of using this approach thoroughly across repos

Any specific concerns?

the required checks in the repo need to be aligned

Yeah, the naming of the checks is annoying. I wonder if Github has a way of setting the name explicitly 🤔 It's causing me trouble elsewhere as well...

Anyways, I tagged a v0.1.0 today - it's also available at v0 and v0.1.

@simoneb
Copy link
Member

simoneb commented Jan 31, 2022

@radomird can you please pick this up again? Thanks

@dominykas
Copy link
Member

dominykas commented Jan 31, 2022

Re build/check naming - I have some changes pending to make these more predictable.

@radomird radomird marked this pull request as ready for review February 23, 2022 10:04
@radomird radomird requested a review from simoneb February 23, 2022 10:07
@radomird radomird merged commit 91d14e0 into master Feb 24, 2022
@radomird radomird deleted the feat/add-pkgjs-action branch February 24, 2022 11:49
@github-actions github-actions bot mentioned this pull request Mar 4, 2022
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.

Feat/add pkgjs workflow
3 participants