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

chore: Update sdk_gen script to lint and not do smoke tests #1431

Merged
merged 2 commits into from
Feb 21, 2024

Conversation

jeremytchang
Copy link
Collaborator

@jeremytchang jeremytchang commented Feb 21, 2024

  • Removed smoke tests as our CI already runs them.
  • Added python and kotlin linting to the script
  • Added yarn fix to the script. For some reason, our pre-commit hook hangs on eslint. So if we first fix the eslint issues with yarn fix this lets our pre-commit hook work.

Additionally:

  • Updated our playbook with additional setup steps for linting
  • Updated our playbook and console output with smoke test debugging steps if CI breaks

Confirmed it generates sdk with consistent styling. From last generation:
26 files changed, 519 insertions(+), 188 deletions(-)

@jeremytchang jeremytchang requested a review from a team as a code owner February 21, 2024 21:29
Copy link
Collaborator

@drstrangelooker drstrangelooker left a comment

Choose a reason for hiding this comment

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

Looks LGTM To Me

Copy link
Contributor

Go Tests

    6 files      6 suites   4m 40s ⏱️
  50 tests   50 ✔️ 0 💤 0 ❌
120 runs  120 ✔️ 0 💤 0 ❌

Results for commit 85f946c.

Copy link
Collaborator

@drstrangelooker drstrangelooker left a comment

Choose a reason for hiding this comment

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

Looks LGTM To Me

@jeremytchang jeremytchang merged commit c687692 into main Feb 21, 2024
12 checks passed
@jeremytchang jeremytchang deleted the update_script branch February 21, 2024 21:35
Comment on lines +196 to +198
'yarn fix', // Lint fix typescript
'pipenv run black python/looker_sdk/sdk/api40/*.py', // Lint fix python
'./kotlin/gradlew -p kotlin spotlessApply', // Lint fix kotlin
Copy link
Contributor

Choose a reason for hiding this comment

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

You gots to do what you gots to do. Would be nice for this to be a one liner to another script, but nice can come later.

@@ -214,6 +216,7 @@ const regen = async (release) => {
const args = process.argv.slice(2);
if (args.length >= 1) {
await regen(args[0]);
console.info('If generated SDKs fail CI, run "bin/smoke [language]" locally to verify and debug')
Copy link
Contributor

Choose a reason for hiding this comment

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

good idea

csharp/sdk/4.0/methods.cs Outdated Show resolved Hide resolved
csharp/sdk/4.0/models.cs Outdated Show resolved Hide resolved
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