-
Notifications
You must be signed in to change notification settings - Fork 17
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
fix(php): format part 2 #443
Conversation
✅ Deploy Preview for api-clients-automation canceled.
|
✗ The generated branch has been deleted.If the PR has been merged, you can check the generated code on the |
.github/.cache_version
Outdated
@@ -1 +1 @@ | |||
9.2 | |||
9.2.1 |
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.
you changed the config file so no need to bump the cache
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.
see #443 (comment), I wanted to try full clear to see if I get the same output
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 have the same effect of clearing the cache, changing the config changes the cache key
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 file should rarely be used now
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.
ahah you didn't had to revert it, it was just an advice
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.
ah actually it does not run codegen without this change :(
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.
ah yes because even though the cache key is different it doesn't set any RUN_
variable, I guess more common files should be added.
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 guess this should be enough d7bdf7c
(I need the codegen to run in this PR)
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 !
scripts/ci/setRunVariables.ts
Outdated
const PHP_CLIENT_FOLDER = getLanguageFolder('php'); | ||
|
||
// Files that are common to every clients | ||
const CLIENTS_COMMON_FILES = ['config/clients.config.json']; |
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.
you could add openapitools.json
to this list
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.
The cache fail was just a fluke ?
Yep... looks like so |
🧭 What and Why
🎟 JIRA Ticket: -
Changes included:
I should've used the
.prettierignore
. Since the spread code script rely on the path, it kinda broke the PHP repo folders.🧪 Test
CI :D