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

ci: add generic version bump and pr testing and relese workflows for nodejs #21

Merged
merged 15 commits into from
Jan 26, 2021

Conversation

derberg
Copy link
Member

@derberg derberg commented Jan 12, 2021

Description

  • these are workflows that will be populated to all repos, even non-nodejs, but will only work if package.json is in the root

version bump

  • the biggest advantage of this one is that PR creation action that we depend on will be updated globally only in this workflow (easier to fix issues like The automated release is failing 🚨 markdown-template#55), and will not be an integral part of individual release workflows
  • we will have to maintain 2 scripts in every nodejs project:
    • generate:assets that will run scripts like the ones about generating docs. toc in readme or types
    • bump:version that will run scripts to bump version of the package in package.json

pr testing

  • changing what nodejs version we are testing in all repos will be as simple as updating just this one single workflow
  • we introduce matrix testing on not only linux but also macos and windows

release workflow
This is what I'm happy with the most, that was not my intention at the beginning but after testing it with React component that had the most complicated release workflow I'm now sure that we can have a simple generic release workflow that can be easily extended per repo with the approach we did in react https://github.com/asyncapi/asyncapi-react/blob/master/.github/workflows/release-wc-and-playground.yml

Related issue(s)
See also asyncapi/spec#423

# We make sure here no tags are added by default as they are already added by release workflow
# --allow-same-version is set in case someone forgot and updated package.json manually and we want to avoide this action to fail and raise confusion
# run: npm --no-git-tag-version --allow-same-version version ${{github.event.release.tag_name}}
run: VERSION=${{github.event.release.tag_name}} npm bump:version
Copy link
Member Author

Choose a reason for hiding this comment

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

it is done this way, with exporting version as environment variable as it is easier this way to later implement npm bump:version script in monorepos

- name: Check if Node.js project and has package.json
id: packagejson
run: test -e ./package.json && "::set-output name=exists::true" || "::set-output name=exists::false"
- if: steps.packagejson.outputs.exists == 'true'
Copy link
Member Author

Choose a reason for hiding this comment

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

yes, we need to put if to every step instead of doing one step with if that checks false and then gracefully stops the job. Why? as github action runner doesn't support that yet :( actions/runner#662

@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

this is what happens to release workflow after removing bump and or creation from it -> asyncapi/html-template#125

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

When this is approved, how will it handle templates which does not contain these new scripts, will it just fail the task @derberg ?

@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

@jonaslagoni it won't handle them gracefully. The job will fail. But it is not a problem as it won't happen 😄
Once this is approved, I know it is a direction we want to take and I have to first go to all repos with the current release pipeline and adjust it:

  • add/adjust scripts
  • remove bump and or creation from releases
  • etc

So this one is not merged until PRs from all the other repos are merged.
I also want to add all those new "global" workflows manually to react component, and make adjustments there and run real release there to make sure all those ideas work as expected

Copy link
Member

@jonaslagoni jonaslagoni left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

@jonaslagoni awesome. What do you think about taking the same approach (generic nodejs workflow with first step that checks if package.json is there) for PR testing and release workflow in general -> https://github.com/asyncapi/html-template/pull/125/files#r555705827

@jonaslagoni
Copy link
Member

@jonaslagoni awesome. What do you think about taking the same approach (generic nodejs workflow with first step that checks if package.json is there) for PR testing and release workflow in general -> https://github.com/asyncapi/html-template/pull/125/files#r555705827

@derberg I dont have any objections in standardize the release process for npm repositories 👍

@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

@jonaslagoni I'll update this PR then, let's see how it will look like

@derberg derberg changed the title chore: add generic version bump workflow for nodejs chore: add generic version bump pr testing and relese workflows for nodejs Jan 12, 2021
@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

ok, now this PR has 3 new global workflows that will run on nodejs projects only.

I don't like this:

      - if: steps.packagejson.outputs.exists == 'true'
        name: Setup Node.js
        uses: actions/setup-node@v1
        with:
          node-version: 14
      - if: steps.packagejson.outputs.exists == 'true'

but there is no alternative now, spent some more hours on investigation. Light in the tunnel for the future is that we could just do the following condition on a job level if: contains(github.event.repository.topics, 'nodejs') and decide that only repo with such topic will apply such a workflow (which is handy) but this is not going to happen soon as topics (even though old feature) are not yet part of repository event 😱 -> https://github.community/t/why-topics-are-not-part-of-repository/155130

@derberg
Copy link
Member Author

derberg commented Jan 12, 2021

@jonaslagoni I updated this PR with generic workflows for release and pr testing. As a result, this is how it will look like in other projects once this PR is merged -> https://github.com/asyncapi/html-template/pull/125/files

Looks promising but as mentioned before now I will do the cleanup in a most complex project, react component

@derberg derberg changed the title chore: add generic version bump pr testing and relese workflows for nodejs ci: add generic version bump pr testing and relese workflows for nodejs Jan 19, 2021
@derberg derberg changed the title ci: add generic version bump pr testing and relese workflows for nodejs ci: Add generic version bump and pr testing and relese workflows for nodejs Jan 19, 2021
@derberg
Copy link
Member Author

derberg commented Jan 19, 2021

@jonaslagoni @magicmatatjahu I updated this PR, including the description. I think it is awesome that we can have generic release workflow for all nodejs projects. All workflows tested against asyncapi-react with success

Of course, this is not going to be merged once I cleanup release workflows across all repos

@derberg derberg merged commit 94c319a into asyncapi:master Jan 26, 2021
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.

3 participants