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

fix: remove deps related to release and switch to npx and non-node config #815

Merged
merged 6 commits into from
Aug 29, 2022
Merged

Conversation

derberg
Copy link
Member

@derberg derberg commented Aug 18, 2022

Description

This PR fixes an issue with generator not able to support node 16 because of issues with release dependencies.
It also is a test of a solution on how we can do release without pacjage.json config and dependencies.

If this works, changes from this repo will go to .github repo and then distributed to all org

  • stop using package.json for release config and switch to .releaserc. This way we will configure releases the same across all repos, no matter the technology behind
  • remove all release-related dependencies from package.json and use semantic-release through npx and install additional missing dependency through release pipeline directly
  • in theory it will make future release configuration for new repos even easier as .releaserc could potentially be stored in .github repo and distributed to selected repos.

Related issue(s)
Fixes #801

@derberg
Copy link
Member Author

derberg commented Aug 18, 2022

semi proof all is good is that CI passes, and previous PRs fail #799

@magicmatatjahu
Copy link
Member

@derberg I cannot understand some things, please clarify them:

stop using package.json for release config and switch to .releaserc. This way we will configure releases the same across all repos, no matter the technology behind

So that file will be distributed to all repos as I guess. I see that we use npm, github etc plugins. In studio and server-api we use also plugin for docker's image publishing. How we will handle that? What if the repo needs a new plugin without changes to .github - currently this problem does not exist, and it seems to me that it will appear after this PR.

remove all release-related dependencies from package.json and use semantic-release through npx and install additional missing dependency through release pipeline directly

I see in this PR that you remove all deps for plugins from package.json but in workflow you only install semantic-release without any additional plugins. Is that correct or you miss something, or I don't understand? I think workflows passed in your PR because you didn't publish - after merge we will have a problems.

Thanks!

@magicmatatjahu
Copy link
Member

magicmatatjahu commented Aug 23, 2022

@derberg I see answer for my second problem:

since few releases of semantic-release, dependencies that we now explicitly add as a dependence (like @semantic-release/github ) are actually included in semantic-release. So we only need to explicitly install only conventional-changelog-conventionalcommits

asyncapi/.github#172

but still, what about additional plugins?

@derberg
Copy link
Member Author

derberg commented Aug 23, 2022

@magicmatatjahu these are very good questions!

So that file will be distributed to all repos as I guess

yes, but only to those that are not having any special modifications. Global workflow that distributes workflows across entire organization supports "selective" approach to workflows distributions. So for example, we won't be able to put generic .releaserc there because it has custom release configuration that also includes upload or artifacts to the github release.

I see that we use npm, github etc plugins. In studio and server-api we use also plugin for docker's image publishing. How we will handle that? What if the repo needs a new plugin without changes to .github - currently this problem does not exist, and it seems to me that it will appear after this PR.

Semantic release package, since few months (wasn't like that at the time the first release pipeline was designed), includes some plugins by default, like:

"@semantic-release/commit-analyzer"
"@semantic-release/release-notes-generator"
"@semantic-release/npm"
"@semantic-release/github"

So ☝🏼 do not have to be installed, as they are there in semantic-release by default when we run it with npx. We only need to explicitly install one plugin, which is conventional-changelog-conventionalcommits. We do it inside workflow now. It works well as we have this approach in go packages already.

For docker:
So for example in this repo, we release to docker using docker directly -> https://github.com/asyncapi/generator/blob/master/.github/workflows/release-docker.yml. It works this way as we wanted to build docker basing on artifact from NPM. Same with github action for generator.

For studio and server-api we have 2 options:

  • main release workflow can always by default install @semantic-release-plus/docker as well. npm run generate:docker would have to run as part of different script, not release. And these projects would have to maintain their own .releaserc with additional config for docker plugin
  • these just switch to generator and generator action approach and release to docker as a separate workflow, as a reaction to release event, or even push but still as a separate workflow. We can turn https://github.com/asyncapi/github-action-for-generator/blob/master/.github/workflows/release-docker.yml into a global workflow too

I see in this PR that you remove all deps for plugins from package.json but in workflow you only install semantic-release without any additional plugins. Is that correct or you miss something, or I don't understand? I think workflows passed in your PR because you didn't publish - after merge we will have a problems.

so new workflow only installs conventional-changelog-conventionalcommits inside workflow, and in case of semantic-release we call it through npx. I explained that other plugins are there already ☝🏼

what I just modified now was to make sure we are installing and calling specific versions of the packages

I think workflows passed in your PR because you didn't publish - after merge we will have a problems.

yes, more important for me in 🟢 CI is that finally tests are passing normally without issues with npm install

@magicmatatjahu
Copy link
Member

@derberg

So ☝🏼 do not have to be installed, as they are there in semantic-release by default when we run it with npx. We only need to explicitly install one plugin, which is conventional-changelog-conventionalcommits. We do it inside workflow now. It works well as we have this approach in go packages already.

Thanks! I realised that when I looked in the asyncapi/.github#172 issue. Thanks!

For studio and server-api we have 2 options:

  • main release workflow can always by default install @semantic-release-plus/docker as well. npm run generate:docker would have to run as part of different script, not release. And these projects would have to maintain their own .releaserc with additional config for docker plugin
  • these just switch to generator and generator action approach and release to docker as a separate workflow, as a reaction to release event, or even push but still as a separate workflow. We can turn https://github.com/asyncapi/github-action-for-generator/blob/master/.github/workflows/release-docker.yml into a global workflow too

Probably second approach will be better. In an ideal world it would be possible to somehow connect to the release process and add plugins, but it would be too much work.

- if: steps.packagejson.outputs.exists == 'true'
name: Publish to any of NPM, Github, and Docker Hub
working-directory: ./.github
Copy link
Member

Choose a reason for hiding this comment

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

My concern: if we run this script in the ./.github directory, will it then see project in the root?

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, working dir is basically set to point where release config file is. It works like a charm with go projects -> https://github.com/asyncapi/parser-go/blob/master/.github/workflows/release.yml#L54 (I just changed location to move it out of workflows dir)

Copy link
Member

Choose a reason for hiding this comment

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

I know that it is done so that the script sees the config file but then will it know where to run the whole release process - in the sense on root? Does it go back to root after reading the config? I'm afraid something might not work.

Copy link
Member Author

Choose a reason for hiding this comment

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

@magicmatatjahu you are right! I assumed all is good but in go projects there is no package json.

I did local dry run and got ENOPKG Missing package.json file.

I made proper changes. There is no way to point to releaserc file with a flag, so we just have to move it to the root

@derberg derberg requested a review from magicmatatjahu August 29, 2022 09:15
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@magicmatatjahu magicmatatjahu 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 Aug 29, 2022

/rtm

@asyncapi-bot asyncapi-bot merged commit b02fea7 into asyncapi:master Aug 29, 2022
@derberg derberg deleted the fixnode16 branch August 29, 2022 13:46
@asyncapi-bot
Copy link
Contributor

🎉 This PR is included in version 1.9.6 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

node and npm version issues
3 participants