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

Set up CI for plugins #10

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Set up CI for plugins #10

merged 4 commits into from
Feb 26, 2024

Conversation

code-asher
Copy link
Member

@code-asher code-asher commented Feb 23, 2024

This will lint, test, build, and publish individual plugins. Workflow details are in the readme; let me know if it seems confusing or if the flow can be improved! At the moment it is basically 1) push a tag 2) publish the release on GitHub (after making sure it looks good).

I had to make some changes to get tsc working (which in turn was required because build told me to run tsc first). See the second commit. I am not too sure about the docs addon change in particular.

Someone with permissions will need to set NPM_TOKEN in the secrets for this repository for publishing to actually work. But I am not sure which token to use. We probably want some bot account. I see we have a coderhq account, maybe we can use that? Need to find out who has access to it, or we can create a new account.

Also, the plugin is marked private so we will have to flip that or yarn will refuse to publish.

Closes https://github.com/coder/backstage.cdr.dev/issues/53
Closes #6

@code-asher code-asher force-pushed the asher/ci branch 23 times, most recently from b373929 to 7954baf Compare February 24, 2024 01:34
This will lint, test, and publish individual plugins.
@code-asher
Copy link
Member Author

code-asher commented Feb 24, 2024

One thing missing from this that I think is important is the changelog. But I think we can tackle that in another PR. I think we can use the "generate release notes" button and clean it up when publishing a release for now, but it would be nice to copy that automatically into a CHANGELOG file (or vice versa).

on:
push:
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

I think that main branch is locked down, but this is probably good to have in here anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

This will trigger after merging PRs to main as well! I am not sure we really need to run tests then but I figured it would be safest just in case it ran in the PR without syncing up with main first.

name: Release

# This workflow will draft a release for a plugin when tagged.
# The tag format is <name>/v<version> without the backstage-plugin- prefix.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# The tag format is <name>/v<version> without the backstage-plugin- prefix.
# The tag format is <name>/v<version> without the backstage-plugin- prefix, e.g. coder/v0.0.0

@@ -39,7 +38,7 @@ class ErrorBoundaryCore extends Component<
}

componentDidCatch(error: Error, errorInfo: ErrorInfo): void {
this.props.onError(error, errorInfo.componentStack);
this.props.onError(error, errorInfo.componentStack || "no details");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Good call with the change. I'm not sure if there are any situations where a component won't naturally have some sort of stack, but always good to have a fallback

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah it seems like one of those things that is never going to happen but the types yell at you anyway. 😆

@Parkreiner
Copy link
Collaborator

I'm not experienced enough in CI to give much valuable feedback, but this all seems to make sense. Also appreciate you cleaning up some of the TypeScript, too!

```sh
cd plugins/backstage-plugin-$name
yarn install
yarn start
Copy link
Member

Choose a reason for hiding this comment

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

Oh is this working now? I thought we were having difficulty running the plugins in isolation for some reason

Copy link
Member Author

@code-asher code-asher Feb 26, 2024

Choose a reason for hiding this comment

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

Oh yeah it starts but it cannot connect to the backend. If we switch to that API factory stuff then I think it should start working, but for now I should put a note that this does not work for the Coder plugin. Or we could delete it, but I wanted to leave it in as a reminder that we should support this flow.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here is what I added:

Note that the Coder plugin does not support running like this as it currently
uses a backend proxy that is not available when running independently of the
Backstage app.

# because the action fetches changed files through the API.
- uses: actions/checkout@v4
if: github.event_name != 'pull_request'
- uses: dorny/paths-filter@v3
Copy link
Member

Choose a reason for hiding this comment

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

I did see we were getting rate-limited on Saturday and I don't know how it could possibly pertain to this PR but just mentioning it because I recognized the action name.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh yeah I saw that too. This action uses the API to figure out the changed files so if we hit the limit this action will fail.

I am not sure if the limit is per-repo or per-org, if the former then we probably have nothing to worry about (I imagine this will have fewer commits than coder/coder) but if the latter then it could be a problem.

It might be worth trying a different action tj-actions/changed-files that can detect changes using only the cloned repository, avoiding the API, if we hit limits here as well.

@Kira-Pilot
Copy link
Member

One thing missing from this that I think is important is the changelog. But I think we can tackle that in another PR.

Good idea! Added a nice-to-have ticket.

Copy link
Member

@Kira-Pilot Kira-Pilot left a comment

Choose a reason for hiding this comment

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

Great work! I learned a lot from reviewing this.

Regarding the tsc change - not sure either. Let's roll with what you've got and see what happens 🤷‍♀️

I made you a repo admin so you'll be able to add the token when the time comes. Kyle is poking into the coderhq account - stay tuned!

@code-asher code-asher merged commit 2cd1098 into main Feb 26, 2024
2 checks passed
@code-asher code-asher deleted the asher/ci branch February 26, 2024 20:14
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.

Publish plugins to NPM
3 participants