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.

Sign up for GitHub

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

chore(ci): update js version bumping strategy #340

Closed
wants to merge 6 commits into from
Closed

chore(ci): update js version bumping strategy #340

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
4dfebf4
chore(ci): update js version bumping strategy
eunjae-lee Apr 5, 2022
81d1ad8
chore: add comment
eunjae-lee Apr 6, 2022
0328424
Merge branch 'main' into chore/js-versioning
eunjae-lee Apr 6, 2022
e480873
Merge branch 'main' into chore/js-versioning
eunjae-lee Apr 6, 2022
c4327eb
Merge branch 'main' into chore/js-versioning
eunjae-lee Apr 6, 2022
8c9fa6f
Merge branch 'main' into chore/js-versioning
eunjae-lee Apr 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 32 additions & 36 deletions scripts/release/process-release.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,10 @@ import {
getOctokit,
} from './common';
import TEXT from './text';
import type {
VersionsToRelease,
BeforeClientGenerationCommand,
BeforeClientCommitCommand,
} from './types';
import type { VersionsToRelease, BeforeClientCommitCommand } from './types';

dotenv.config({ path: ROOT_ENV_PATH });

const BEFORE_CLIENT_GENERATION: {
[lang: string]: BeforeClientGenerationCommand;
} = {
javascript: async ({ releaseType, dir }) => {
await run(`yarn release:bump ${releaseType}`, { cwd: dir });
},
};

const BEFORE_CLIENT_COMMIT: { [lang: string]: BeforeClientCommitCommand } = {
javascript: async ({ dir }) => {
// https://github.com/yarnpkg/berry/issues/2948
Expand Down Expand Up @@ -194,29 +182,10 @@ async function processRelease(): Promise<void> {

const issueBody = await getIssueBody();
const versionsToRelease = getVersionsToRelease(issueBody);

await updateOpenApiTools(versionsToRelease);

const langsToRelease = Object.keys(versionsToRelease);

for (const lang of langsToRelease) {
const generateAndUpdateChangelog = async (lang: string): Promise<void> => {
const { current, releaseType } = versionsToRelease[lang];
/*
About bumping versions of JS clients:

There are generated clients in JS repo, and non-generated clients like `algoliasearch`, `client-common`, etc.
Now that the versions of generated clients are updated in `openapitools.json`,
the generation output will have correct new versions.

However, we need to manually update versions of the non-generated (a.k.a. manually written) clients.
In order to do that, we run `yarn release:bump <releaseType>` in this monorepo first.
It will update the versions of the non-generated clients which exists in this monorepo.
After that, we generate clients with new versions. And then, we copy all of them over to JS repository.
*/
await BEFORE_CLIENT_GENERATION[lang]?.({
releaseType,
dir: toAbsolutePath(getLanguageFolder(lang)),
});

console.log(`Generating ${lang} client(s)...`);
console.log(await run(`yarn cli generate ${lang}`));
Expand All @@ -228,8 +197,34 @@ async function processRelease(): Promise<void> {
current,
next: next!,
});
};

// 1. Generate JS clients first
// We generate javascript packages BEFORE updating the versions in `openapitools.json`,
// because we bump the versions with lerna after the generation.
// The reason behind this is that it's hard to update the versions of packages
// and the dependencies correctly, due to the limitation of templating.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could the limitation of templating be overcome with the custom generator ? If it about passing an object with all version to the template it's doable, unless it's easier to do it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To be precise, we need the new versions of client-common, and requesters packages, and pass them to each client (client-analytics, client-...), so that those dependencies will have correct version. I haven't touched the custom generator, but I believe it's doable. However, I didn't explore that path, because it involves too much "custom" handling. (This PR introduces another type of "custom" code, though 😅)

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah I don't know which one is best, we need to keep all the custom code in one place and not spread a lot but it's already done ahah

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, but the reason why I didn't explore that custom generator is, because bumping version can be really tricky. I already did the work previously in this repo, but it didn't work due to edge cases like this. And lerna version is actually a good tool to take care of everything out of the box. So the direction in this PR is to maximise the benefit of lerna version instead of doing much custom thing here.

Copy link
Member

Choose a reason for hiding this comment

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

What if the JavaScript openapitools object include a new variable for the dependency version, that we can use in the template? So the updateOpenApiTools update it with the correct bump and we can then generate without making exceptions

Copy link
Member

Choose a reason for hiding this comment

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

it indeed adds a new variable, but it's fully controlled by the script and have the minimal footprint

Copy link
Contributor Author

@eunjae-lee eunjae-lee Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@shortcuts That's an idea. It means we need to add something like

      "javascript-search": {
        "generatorName": "algolia-javascript",
        "templateDir": "#{cwd}/templates/javascript/",
        "config": "#{cwd}/openapitools.json",
        "apiPackage": "src",
        "output": "#{cwd}/clients/algoliasearch-client-javascript/packages/client-search",
        "glob": "specs/bundled/search.yml",
        "gitHost": "algolia",
        "gitUserId": "algolia",
        "gitRepoId": "algoliasearch-client-javascript",
        "reservedWordsMappings": "queryParameters=queryParameters,requestOptions=requestOptions",
        "additionalProperties": {
          "modelPropertyNaming": "original",
          "supportsES6": true,
          "npmName": "@experimental-api-clients-automation/client-search",
          "buildFile": "client-search",
          "apiName": "search",
          "capitalizedApiName": "Search",
          "packageVersion": "0.0.5",
          "packageName": "@experimental-api-clients-automation/client-search"
+         "clientCommonPackageVersion": "0.0.5",
+         "clientRequesterNodePackageVersion": "0.0.5",
+         "clientRequesterBrowserPackageVersion": "0.0.5"
        }
      },

to every javascript-xxx generator. Then, we'll be able to use those variables in package.mustache.

What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Since those are internal package it's fine to sync their versions and only have one value IMO, this way we could even introduce new ones without changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to this suggestion, I opened a new PR #346 and it seems much cleaner! Thanks. Closing this PR.

if (langsToRelease.includes('javascript')) {
const lang = 'javascript';
const { releaseType } = versionsToRelease[lang];

await generateAndUpdateChangelog(lang);

console.log(`Bumping versions of ${lang} client(s)...`);
await run(`yarn release:bump ${releaseType}`, {
cwd: toAbsolutePath(getLanguageFolder(lang)),
});
}

// 2. Update the versions in `openapitools.json`
await updateOpenApiTools(versionsToRelease);

// 3. Generate the rest of languages
for (const lang of langsToRelease.filter((l) => l !== 'javascript')) {
await generateAndUpdateChangelog(lang);
}

// 4. Push to each repository
// We push commits to each repository AFTER all the generations are done.
// Otherwise, we will end up having broken release.
for (const lang of langsToRelease) {
Expand Down Expand Up @@ -259,21 +254,22 @@ async function processRelease(): Promise<void> {
await run(`git push --follow-tags`, { cwd: tempGitDir });
}

// Commit and push from the monorepo level.
// 5. Commit and push from the monorepo level.
await configureGitHubAuthor();
await run(`YARN_ENABLE_IMMUTABLE_INSTALLS=false yarn install`);
await run(`git add .`);
const dateStamp = getDateStamp();
await gitCommit({
message: `chore: release ${dateStamp}`,
});
await run(`git push`);

// remove old `released` tag
// 6. remove old `released` tag
await run(`git fetch origin refs/tags/released:refs/tags/released`);
await run(`git tag -d ${RELEASED_TAG}`);
await run(`git push --delete origin ${RELEASED_TAG}`);

// create new `released` tag
// 7. create new `released` tag
await run(`git tag released`);
await run(`git push --tags`);
}
Expand Down
5 changes: 0 additions & 5 deletions scripts/release/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,6 @@ export type VersionsToRelease = {
};
};

export type BeforeClientGenerationCommand = (params: {
releaseType: ReleaseType;
dir: string;
}) => Promise<void>;

export type BeforeClientCommitCommand = (params: {
dir: string;
}) => Promise<void>;