-
Notifications
You must be signed in to change notification settings - Fork 18
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
chore(ci): push changes to submodules #155
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to configure something on the .git
side for the submodules? If so, could you add a doc?
const clientPath = path.resolve( | ||
ROOT_DIR, | ||
'clients/dummy-algoliasearch-client-javascript' | ||
); | ||
const runInClient: Run = (command, options = {}) => | ||
runOriginal(command, { | ||
cwd: clientPath, | ||
...options, | ||
}); | ||
|
||
runInClient(`git checkout next`); | ||
run( | ||
`cp -r clients/algoliasearch-client-javascript/ clients/dummy-algoliasearch-client-javascript` | ||
); | ||
runInClient(`git add .`); | ||
execa.sync('git', ['commit', '-m', 'chore: release test'], { | ||
cwd: clientPath, | ||
}); | ||
runInClient(`git push origin next`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we have all the client infos of the openapitools.json
file, I'd rather see values retrieved from here (either with new additionalProperties
or paths etc.) so we keep the single source of truth and avoid having to add hardcoded values
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it will change, but this is a hard-coded test for a temporary period, because I need something to be merged, in order to actually test it. And that's why I chose js client and using dummy repo as a submodule.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok make sense, an alternative could be to create a branch and update this part https://github.com/algolia/api-clients-automation/blob/main/.github/workflows/check.yml#L8
So you don't have to merge test things or main, and mostly you don't have to wait for reviews
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good idea! instead of that, I think this is the way https://github.com/algolia/api-clients-automation/pull/155/files#r812729664
Cloud you grant access to the team to the dummy repo? |
done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice :D
I think it make sense to consider this comment https://github.com/algolia/api-clients-automation/pull/155/files#r812670890 to make iteration on this release easier, but it's up to you!
…omation into chore/submodules
@@ -17,6 +17,9 @@ jobs: | |||
- uses: actions/checkout@v2 | |||
with: | |||
fetch-depth: 0 | |||
submodules: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I should've set this (ref: https://github.com/actions/checkout)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice
@@ -17,6 +17,9 @@ jobs: | |||
- uses: actions/checkout@v2 | |||
with: | |||
fetch-depth: 0 | |||
submodules: true | |||
|
|||
- run: git checkout chore/release |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will enable me to push test commits to this test branch and run the test without opening numerous PRs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's hard to determine if it will work, but it looks correct!
Let's try out and see if it works! (I have no idea either lol) |
* chore: add dummy submodule * chore(ci): push change to submodule * chore: run release script on test branch
🧭 What and Why
Changes included:
process-release.ts
)For now, we're using this dummy repository. Once confirmed working, there will be a separate PR to put things properly.