fix(ci): make generation comment clearer #258
Conversation
|
✔️ Deploy Preview for api-clients-automation canceled. 🔨 Explore the source changes: d7abc04 🔍 Inspect the deploy log: https://app.netlify.com/sites/api-clients-automation/deploys/6231f09cc617ab0009b15bfd |
b5adceb to
da2a9d8
Compare
|
✗ The generated branch has been deleted. If the PR has been merged, you can check the generated code on the |
| @@ -2,6 +2,11 @@ name: Setup | |||
|
|
|||
| description: Setup CI environment. | |||
|
|
|||
| inputs: | |||
There was a problem hiding this comment.
Because this file is quite different with minimal, is it worth it to create another file instead ?
There was a problem hiding this comment.
I'm not sure how nested composite would behave but splitting the setup file would definitely make sense. Might be worth investigating that
| describe('cleanGeneratedBranch', () => { | ||
| it('throws without parameters', async () => { | ||
| await expect( | ||
| // @ts-expect-error a parameter is required |
There was a problem hiding this comment.
You can pass undefined to remove the @ts-expect-error
There was a problem hiding this comment.
It wouldn't be accurate as we actually don't accept undefined, it's only here to make sure it throws
| if (trigger === 'notification' || trigger === 'noGen') { | ||
| return `${commentText[trigger].header} | ||
|
|
||
| ${commentText[trigger].body}`; |
There was a problem hiding this comment.
Maybe body could always be a function to remove this if.
| const REPO_URL = `https://github.com/${OWNER}/${REPO}`; | ||
|
|
||
| export default { | ||
| notification: { |
There was a problem hiding this comment.
Can you give those object a type please ?
There was a problem hiding this comment.
Inferred type is enough here, we don't use it elsewhere
There was a problem hiding this comment.
I was saying that to have the same type between all objects, like all body should be function
There was a problem hiding this comment.
Ah ok I see, I'm not sure it's necessary as it does not need a parameter. If you would like to make it consistent I have nothing against it
There was a problem hiding this comment.
I think having the same type in the array make it more predictable and easier to maintain
🧭 What and Why
🎟 JIRA Ticket: https://algolia.atlassian.net/browse/APIC-378
Changes included:
codegenCI🧪 Test