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

[Discussion] Two functionalities in one action; complex use cases unsupported #426

Open
trueberryless opened this issue Nov 29, 2024 · 0 comments

Comments

@trueberryless
Copy link

Disclaimer: This behaviour is probably very intended!

TL;DR

This action tries to combine two functionalities into a single action which works for 99% of use cases. As long as you only want to publish npm packages in a monorepo or single repo, everything works fine, no complains at all. But if you want to only have one of the two functionalities in one action, you're screwed. (You could skip the Problem section right away, it just explains in detail.)

Problem

I am a fan of the Single Responsibility Principle. Therefore, when I create GitHub Actions, I always try to separate steps into jobs and jobs into multiple actions (files). This keeps the code clean and easy to read, because every job has a specific job and every action does entirely different actions. Pun intended.

However, the changeset action prevents me from achieving the Single Responsibility Principle because if I choose to use the action in multiple actions (reasons explained below) every instance of it must currently push changes to the PR (if a changeset is detected) because the commit and title options default to Version Packages. This is especially confusing because this action claims to be focused on monorepos (altho changesets/changesets#849 already enhances this tagline).

Let's assume we want to create a GitHub monorepo which not only publishes npm packages, but also wants to build an Astro site when a Version Packages PR gets merged.

As I mentioned before, the optimal way imho to manage and maintain such complex scenarios is splitting the different tasks into different action files:

├── .github
│   └── workflows
│       ├── release.yaml
│       ├── publish.yaml
│       ├── deployment.yaml
  • The release.yaml action file has the responsibility of creating and maintaining the Version Packages PR
  • The publish.yaml action publishes a package to npm if hasChangesets == 'false' (the recommended way)
  • The deployment.yaml action builds the Astro site and somehow (not important in this context) deploys the site if hasChangesets == 'false' (the recommended way)

Notice that both, the publish.yaml and the deployment.yaml action, need the information hasChangesets from the changeset action in order to confirm if a new release should occur. This necessarily means that they both need to run the changeset action beforehand. Running the changeset action necessarily means that new commits will be force pushed to the Version Packages PR if a changeset has been detected. This functionality has NOTHING to do with the actual release.

And here comes my actual issue with the whole action: It tries to combine two functionalities into a single action. The complete opposite of my beloved Single Responsibility Principle. This is completely fine for 99% of use cases (monorepos which only need to publish npm packages and want to get started quickly), but it bugs me to death if you want to do anything else or more complex. In other words: Custom publishing isn't really supported by this action.

Possible Solutions

Make Create PR optional

The one possibility to quickly solve this specific issue would be to create an additional input (for example skipCreatePr) which enables a whole new use case for the changeset action because the user could choose what functionality they want to use now:

  • If you want to use the action in order to create a PR (release.yaml action in example explained in Problem) it would look like this:

       - name: Create Release Pull Request
          uses: changesets/action@v1
          with:
            version: pnpm run version
          env:
            GITHUB_TOKEN: ${{ secrets.PUBLIC_GITHUB_TOKEN }}
    
  • If you want to use the action in order to run a new release (publish.yaml or deployment.yaml in example explained in Problem) it would look like this:

       - name: Create Release Pull Request
          id: changesets
          uses: changesets/action@v1
          with:
            skipCreatePr: true
          env:
            GITHUB_TOKEN: ${{ secrets.PUBLIC_GITHUB_TOKEN }}
    

    and afterwards you could use the hasChangesets information like this:

    publish:
      needs: [changesets]
      if: needs.changesets.outputs.hasChangesets == 'false'
    

    Note that the if statement needs probably to be more complex, like explained in the changeset action docs:

    Note that you might need to account for things already being published in your script because a commit without any new changesets can always land on your base branch after a successful publish. In such a case you need to figure out on your own how to skip over the actual publishing logic or handle errors gracefully as most package registries won't allow you to publish over already published version.

Split the action into two actions

DISCLAIMER: This is not going to happen!

I personally would be very happy to see two separate actions where:

  • one action handles the creation of the PR
  • and the other action handles the publishing (or at least provides the hasChangesets output so a custom publishing is possible)

Because this is a major change, I of course do not expect do not even want that such a change is integrated into this repo. Rather I want to collect information how many people are actually bumping into the same issue in general and if it is worth it to create a new kinda branch which goes into a completely new direction. Feel free to comment below what you think about the whole topic and the future of this action.

Related issues

changesets/changesets#849
#422

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

No branches or pull requests

1 participant