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

Fetch release-please manifest and config files from changes-branch #31

Merged
merged 7 commits into from
Sep 8, 2023

Conversation

dgellow
Copy link
Member

@dgellow dgellow commented Sep 8, 2023

When using release-please with --change-branch=next against a new repository where no manifest or config has been merged to main yet, we get an error:

/home/sam/stainless/stainless/gh-app/node_modules/@stainless-api/release-please/src/manifest.ts:1596
      throw new ConfigurationError(
            ^
ConfigurationError: base (DefinitelyATestOrg/sdk-node): Missing required manifest config: release-please-config.json
    at fetchManifestConfig (/home/sam/stainless/stainless/gh-app/node_modules/@stainless-api/release-please/src/manifest.ts:1596:13)
    at async parseConfig (/home/sam/stainless/stainless/gh-app/node_modules/@stainless-api/release-please/src/manifest.ts:1541:18)
    at async Promise.all (index 0)
    at async Function.fromManifest (/home/sam/stainless/stainless/gh-app/node_modules/@stainless-api/release-please/src/manifest.ts:417:9)
    at async runReleasePlease (/home/sam/stainless/stainless/gh-app/src/helpers/run-release-please.ts:47:20) {
  releaserName: 'base',
  repository: 'DefinitelyATestOrg/sdk-node'
}

My test repositor has a next branch with a manifest, but no release has been created yet, so main is just an "Initial commit".

The way upstream release-please generally work is that a release-please bootstrap command is first run to generate a default manifest and config. But in the context of Stainless that step isn't necessary, content pushed to next will always have release-please config files, we just need to be sure release-please is looking at the correct branch. Given that next is always supposed to be based on top of main we can consider its manifest as up to date.

@dgellow
Copy link
Member Author

dgellow commented Sep 8, 2023

That will benefit both Stainless CI (Robert faced this issue) and the GitHub App (that's where I personally faced the issue when running against my freshly created test repo).

@dgellow
Copy link
Member Author

dgellow commented Sep 8, 2023

You can see the generated PR here: DefinitelyATestOrg/sdk-node#1

@@ -751,7 +752,7 @@ If you instead want to use the version number \`${newVersion}\` generated from c
return Version.parse(this.initialVersion);
}

return Version.parse('1.0.0');
return Version.parse('0.0.1');
Copy link
Member Author

Choose a reason for hiding this comment

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

That's more in line with initial version numbers Stainless uses.

package.json Show resolved Hide resolved
Comment on lines -199 to -201
protected initialReleaseVersion(): Version {
return Version.parse('0.1.0');
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure why the initial version number is different for different languages? That's kind of odd and doesn't match Stainless behaviour.

@dgellow
Copy link
Member Author

dgellow commented Sep 8, 2023

@RobertCraigie Unless that's blocking you I will wait for Marcel to be able to review before merging. Just in case he disagrees or has some feedback.

Edit: it's not a blocker for my own work, I just updated my stuff to target this branch.

@dgellow dgellow merged commit 8c1a45c into main Sep 8, 2023
5 checks passed
@dgellow dgellow deleted the sam/look-for-manifest-from-changes-branch branch September 8, 2023 18:26
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