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: noStreams skips tally headers in generated source #727

Merged
merged 2 commits into from
Jul 1, 2021
Merged

Conversation

jkaster
Copy link
Contributor

@jkaster jkaster commented Jul 1, 2021

When the -n option is used for the code generator, the tally comments at the top of each generated source file are also removed.

Consider it "no comment" as well as "no streams"

When the `-n` option is used for the code generator, the tally comments at the top of each generated source file are also removed.

Consider it "no comment" as well as "no streams"
@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2021

Codegen Tests

  1 files  ±0    6 suites  ±0   23s ⏱️ ±0s
58 tests ±0  46 ✔️ ±0  12 💤 ±0  0 ❌ ±0 

Results for commit e06a3d6. ± Comparison against base commit 1a835a8.

whscullin
whscullin previously approved these changes Jul 1, 2021
Copy link

@whscullin whscullin left a comment

Choose a reason for hiding this comment

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

One q, otherwise LGTM

// this.p(strs)
// }
// return this
// }

Choose a reason for hiding this comment

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

dead code?

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. I'll remove it in an update

@github-actions
Copy link
Contributor

github-actions bot commented Jul 1, 2021

Codegen Tests

  1 files  ±0    6 suites  ±0   20s ⏱️ -3s
58 tests ±0  46 ✔️ ±0  12 💤 ±0  0 ❌ ±0 

Results for commit 0c62dd9. ± Comparison against base commit 1a835a8.

@jkaster jkaster requested a review from whscullin July 1, 2021 04:12
Copy link

@whscullin whscullin left a comment

Choose a reason for hiding this comment

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

LGTM

@jkaster jkaster merged commit 113c2c5 into main Jul 1, 2021
@jkaster jkaster deleted the jk/tally_ho branch July 1, 2021 16:00
@github-actions

This comment has been minimized.

@joeldodge79
Copy link
Contributor

is there a follow on generated code change? also, fwiw this change will not show up in any changelog nor, in and of itself, cause release-please to recognize any need for a version bump (chore:...)

@jkaster
Copy link
Contributor Author

jkaster commented Jul 1, 2021

There is no code change for the external SDK. Let me know how to fix it so it gets released, please.

@joeldodge79
Copy link
Contributor

released, please.

actually looks like sdk-codegen-scripts is already in the current release-pr due to a dependency update. But there's no changelog entry for it. If you don't mind about the missing changelog entry then we're good.

if you do want a changelog entry then we can add a temporary release-please config entry (under "packages/sdk-codegen-scripts"):

+      "changelog-sections": [
+        {"type": "chore", "section": "Bug Fixes"}
+      ]

let me know if you want that (or feel free to put up a PR for release-please-config.json)

@jkaster
Copy link
Contributor Author

jkaster commented Jul 1, 2021

It's really for internal use, so the changelog doesn't matter. Thanks.

joeldodge79 added a commit that referenced this pull request Jul 1, 2021
temp override changelog types for sdk-codegen-scripts for #727
set next versions on non-standard packages
joeldodge79 added a commit that referenced this pull request Jul 1, 2021
temp override changelog types for sdk-codegen-scripts for #727
set next versions on non-standard packages
jkaster pushed a commit that referenced this pull request Jul 1, 2021
🤖 I have created a release \*beep\* \*boop\*
---
<details><summary>sdk-codegen-all: 1.9.1</summary>


### Bug Fixes

* paging parsing issues ([#728](https://www.github.com/looker-open-source/sdk-codegen/issues/728)) ([f8eec43](https://www.github.com/looker-open-source/sdk-codegen/commit/f8eec43bdfbe337d41b1da02c127d690c8815ed3))
</details>
<details><summary>@looker/sdk-codegen-scripts: 21.0.20</summary>


### Bug Fixes

* noStreams skips tally headers in generated source ([#727](https://www.github.com/looker-open-source/sdk-codegen/issues/727)) ([113c2c5](https://www.github.com/looker-open-source/sdk-codegen/commit/113c2c50c07c621cf94841af75557704fc3f5df7))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19
    * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/sdk-rtl: 21.0.15</summary>


### Bug Fixes

* paging parsing issues ([#728](https://www.github.com/looker-open-source/sdk-codegen/issues/728)) ([f8eec43](https://www.github.com/looker-open-source/sdk-codegen/commit/f8eec43bdfbe337d41b1da02c127d690c8815ed3))
</details>
<details><summary>@looker/sdk: 21.8.2</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/sdk-codegen: 21.0.19</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/extension-sdk: 21.8.2</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/sdk-node: 21.8.1</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/code-editor: 0.1.4</summary>


### Dependencies

* The following workspace dependencies were updated
  * devDependencies
    * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19
</details>
<details><summary>@looker/extension-sdk-react: 21.8.2</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/wholly-sheet: 0.5.10</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
  * devDependencies
    * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1
</details>
<details><summary>@looker/run-it: 0.9.11</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19
    * @looker/code-editor bumped from ^0.1.3 to ^0.1.4
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
</details>
<details><summary>@looker/hackathon: 21.8.1</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/extension-sdk-react bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
    * @looker/wholly-sheet bumped from ^0.5.9 to ^0.5.10
</details>
<details><summary>@looker/api-explorer: 0.9.11</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/code-editor bumped from ^0.1.3 to ^0.1.4
    * @looker/run-it bumped from ^0.9.10 to ^0.9.11
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19
    * @looker/sdk-rtl bumped from ^21.0.14 to ^21.0.15
  * devDependencies
    * @looker/sdk-codegen-scripts bumped from ^21.0.19 to ^21.0.20
    * @looker/sdk-node bumped from ^21.8.0 to ^21.8.1
</details>
<details><summary>@looker/extension-api-explorer: 21.8.1</summary>


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * @looker/api-explorer bumped from ^0.9.10 to ^0.9.11
    * @looker/extension-sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/extension-sdk-react bumped from ^21.8.1 to ^21.8.2
    * @looker/run-it bumped from ^0.9.10 to ^0.9.11
    * @looker/sdk bumped from ^21.8.1 to ^21.8.2
    * @looker/sdk-codegen bumped from ^21.0.18 to ^21.0.19
</details>


This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants