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

separate-pull-requests + node-workspace plugin issues #1858

Closed
monholm opened this issue Feb 20, 2023 · 5 comments · May be fixed by #2310
Closed

separate-pull-requests + node-workspace plugin issues #1858

monholm opened this issue Feb 20, 2023 · 5 comments · May be fixed by #2310
Assignees
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@monholm
Copy link

monholm commented Feb 20, 2023

While trying to use release-please with separate-pull-requests and the node-workspace plugin enabled, I stumbled upon a couple of issues and some odd behaviour.

When pushing a commit for a single (and at this point in the repro, the only) package, in addition to the chore(main): release <package-name> x.y.z pr, release-please opens another pr (chore: release main) with 0 files changed. The chore: release main pr is open constantly and contains 0 files changed.

Is this expected behaviour? If so, how is one supposed to treat this PR? Leave it as is, merge it before or after the pr containing actual changes, or? I was unable to find any documentation regarding this, so I'm guessing this might be a bug? :)

Moving on from there, using the same setup, if one updates a package that's referenced in another package, the chore: release main pr is updated with the patch bump change for the package that references the updated package. But the changelog change is invalid and if one tries to merge the PR, no tag is created for the patch bump, causing the versioning to fail. I've created a repro, as this is quite difficult describing... 😅

Environment details

  • OS: Ubuntu 22.04
  • Node.js version: v18.14.1
  • npm version: v9.3.1
  • release-please version: v15.7.0

Steps to reproduce

  1. Setup basic manifest-based project
  2. Enable separate-pull-request and node-workspace plugin
  3. Add single package and push commit to trigger first release
  4. Observe two PR's being opened, one of which is empty
  5. Add another package and make it depend on the first package.
  6. Update the first package and observe the invalid changelog added to the previously empty PR
  7. If the PR mentioned in step 6 is merged, no tag is created for the bumped package

Again, I've created a repro that hopefully makes it easier to grasp. If not, let me know if I can help provide any other info :)

@monholm monholm added priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. labels Feb 20, 2023
@lorentzfb
Copy link

I have the same issues

@SurferJeffAtGoogle SurferJeffAtGoogle added priority: p3 Desirable enhancement or fix. May not be included in next release. and removed priority: p2 Moderately-important priority. Fix may not be included in next release. labels Jun 5, 2023
@shinebayar-g
Copy link

shinebayar-g commented Aug 24, 2023

Having an exact same issue. release-please is creating additional empty PR besides the actual package PR.
image

Also there is no autorelease: tag attached to the PR.

release-please-config.json

{
  "$schema": "https://raw.githubusercontent.com/googleapis/release-please/main/schemas/config.json",
  "bootstrap-sha": "942d93a2423b28394b427eabdae1b2cc7a174742",
  "bump-minor-pre-major": true,
  "bump-patch-for-minor-pre-major": false,
  "include-v-in-tag": true,
  "separate-pull-requests": true,
  "sequential-calls": true,
  "packages": {
    ".github": {
      "package-name": "workflows",
      "release-type": "simple"
    },
    "packages/my-package": {
      "package-name": "my-package",
      "release-type": "node"
    }
  },
  "plugins": ["node-workspace"],
  "changelog-sections": [
    { "type": "feat", "section": "Features", "hidden": false },
    { "type": "fix", "section": "Bug Fixes", "hidden": false },
    { "type": "chore", "section": "Miscellaneous", "hidden": false },
    { "type": "deps", "section": "Dependencies", "hidden": false },
    { "type": "docs", "section": "Documentation", "hidden": false },
    { "type": "ci", "section": "Continuous Integration", "hidden": false }
  ]
}

.release-please-manifest.json

{
  ".github": "2.3.1",
  ".github+FILLER": "0.0.0",
  "packages/my-package": "2.3.0",
  "packages/my-package+FILLER": "0.0.0"
}

@Badisi
Copy link

Badisi commented Apr 6, 2024

Also facing the exact same issue.

Sad to see that it was reported a year ago and still hasn't been fixed 😕..

@Badisi
Copy link

Badisi commented Apr 10, 2024

I was able to track down the issue to the merge plugin (used when enabling the node-workspace plugin).

@chingor13, I don't really understand the purpose of the code here, so I will let you decide what needs to be fixed.
Thanks 😊


Trace

From manifest.ts#buildPullRequests() the node-workspace plugin is called:

for (const plugin of this.plugins) {
this.logger.debug(`running plugin: ${plugin.constructor.name}`);
newReleasePullRequests = await plugin.run(newReleasePullRequests);
}

The node-workspace plugin inherits from WorkspacePlugin but does not overrides the run() method.

So workspace.ts#run() is called instead, and there, the merge plugin is called:

if (this.merge) {
this.logger.info(`Merging ${newCandidates.length} in-scope candidates`);
const mergePlugin = new Merge(
this.github,
this.targetBranch,
this.repositoryConfig
);
newCandidates = await mergePlugin.run(newCandidates);
}

Here, the merge.ts#run() will always return a PR for the root project merged with the candidates:

return [
{
path: ROOT_PROJECT_PATH,
pullRequest,
config: {
releaseType,
},
},
...outOfScopeCandidates,
];

Resulting, for a mono-repo, to always have an empty chore: release main even though there is no project at the root.

@marjorg
Copy link

marjorg commented Aug 8, 2024

@chingor13 I might be wrong, but this does not seem to be resolved. The referenced PR above is not merged and has been inactive for ~two months. Has this been resolved elsewhere?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: p3 Desirable enhancement or fix. May not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants