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.

Sign up for GitHub

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

Jump to bottom

fix: return all library names if repo-level parameter changes #3379

Merged
merged 4 commits into from
Nov 18, 2024
Merged

fix: return all library names if repo-level parameter changes #3379

merged 4 commits into from
Nov 18, 2024

Conversation

JoeWang1127
Copy link
Collaborator

@JoeWang1127 JoeWang1127 commented Nov 15, 2024 •
edited
Loading

In this PR:

  • Return all library names as a comma-separated string if repo-level parameter changes.

Fix: #3381

@product-auto-label product-auto-label bot added the size: s Pull request size is small. label Nov 15, 2024
@JoeWang1127 JoeWang1127 marked this pull request as ready for review November 15, 2024 21:42
blakeli0
blakeli0 reviewed Nov 18, 2024
View reviewed changes
hermetic_build/common/model/config_change.py Outdated

:return: library names of change libraries.
"""
if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries:
return ConfigChange.ALL_LIBRARIES_CHANGED
return [ConfigChange.ALL_LIBRARIES_CHANGED]
Copy link
Collaborator

Choose a reason for hiding this comment

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

The previous behavior was that we return a list if there are changed libraries or a String ConfigChange.ALL_LIBRARIES_CHANGED if there is no changes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The previous behavior:

  • Return None if all libraries changed
  • String of library names if some libraries changed
  • Empty string if no library changed

The problem is None is interpreted as empty string in the shell script.

hermetic_build/common/model/config_change.py Outdated

:return: library names of change libraries.
"""
if ChangeType.REPO_LEVEL_CHANGE in self.change_to_libraries:
return ConfigChange.ALL_LIBRARIES_CHANGED
return [ConfigChange.ALL_LIBRARIES_CHANGED]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we return all library names if ChangeType.REPO_LEVEL_CHANGE? We don't have to add a magic String just for all libraries changes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, I think this is more intuitive than this magic string. I'll look into it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'gapic-generator-java-root'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed for 'java_showcase_integration_tests'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@JoeWang1127 JoeWang1127 requested a review from blakeli0 November 18, 2024 21:35
@blakeli0
Copy link
Collaborator

Please update the PR title and description to be more specific before merging.

@JoeWang1127 JoeWang1127 changed the title fix: return a non null list fix: return all library names if repo-level parameter changes Nov 18, 2024
@JoeWang1127 JoeWang1127 merged commit 75dcb96 into main Nov 18, 2024
49 of 55 checks passed
@JoeWang1127 JoeWang1127 deleted the fix/return-non-null-list branch November 18, 2024 22:09
@release-please release-please bot mentioned this pull request Nov 18, 2024
Merged
JoeWang1127 pushed a commit that referenced this pull request Dec 12, 2024
@release-please
chore(main): release 2.51.0 (#3382)
1e73a4a 
🤖 I have created a release *beep* *boop*
---


<details><summary>2.51.0</summary>

##
[2.51.0](v2.50.0...v2.51.0)
(2024-12-12)


### Features

* [iam] add ResourcePolicyMember to google/iam/v1
([b8e2859](b8e2859))


### Bug Fixes

* [#3381](#3381)
([75dcb96](75dcb96))
* graalvm missing build time class when using protobuf 4.x
([#3438](#3438))
([f0236cf](f0236cf))
* return all library names if repo-level parameter changes
([#3379](#3379))
([75dcb96](75dcb96)),
closes
[#3381](#3381)


### Dependencies

* update dependency com.fasterxml.jackson:jackson-bom to v2.18.2
([#3422](#3422))
([fdc3a54](fdc3a54))
* update dependency com.google.api-client:google-api-client-bom to
v2.7.1
([#3450](#3450))
([35f1310](35f1310))
* update dependency com.google.auth:google-auth-library-oauth2-http to
v1.30.0
([#3428](#3428))
([78cbff1](78cbff1))
* update dependency com.google.errorprone:error_prone_annotations to
v2.36.0
([#3425](#3425))
([52dcc0d](52dcc0d))
* update dependency com.google.errorprone:error_prone_annotations to
v2.36.0
([#3426](#3426))
([77dd85d](77dd85d))
* update dependency dev.cel:cel to v0.8.0
([#3429](#3429))
([79cde20](79cde20))
* update dependency io.github.java-diff-utils:java-diff-utils to v4.15
([#3430](#3430))
([91fd5cb](91fd5cb))
* update dependency net.bytebuddy:byte-buddy to v1.15.10
([#3298](#3298))
([7b503de](7b503de))
* update dependency org.checkerframework:checker-qual to v3.48.3
([#3278](#3278))
([c5fd1b4](c5fd1b4))
* update dependency org.easymock:easymock to v5.5.0
([#3431](#3431))
([3c22f5e](3c22f5e))
* update dependency packaging to v24.2
([#3432](#3432))
([c1e7c81](c1e7c81))
* update docker.io/library/alpine docker tag to v3.21.0
([#3433](#3433))
([c8318c2](c8318c2))
* update docker.io/library/alpine:3.20.3 docker digest to 1e42bbe
([#3417](#3417))
([0f4ef19](0f4ef19))
* update docker.io/library/maven:3.9.9-eclipse-temurin-11-alpine docker
digest to 3bab9f2
([#3447](#3447))
([ccf7eac](ccf7eac))
* update docker.io/library/maven:3.9.9-eclipse-temurin-11-alpine docker
digest to cdfb386
([#3418](#3418))
([2559ff8](2559ff8))
* update docker.io/library/python docker tag to v3.13.1
([#3434](#3434))
([feefd27](feefd27))
* update google api dependencies
([#3435](#3435))
([3a1e8f2](3a1e8f2))
* update google auth library dependencies to v1.30.1
([#3453](#3453))
([d7b7dd9](d7b7dd9))
* update google http client dependencies to v1.45.2
([#3394](#3394))
([f8ee892](f8ee892))
* update google http client dependencies to v1.45.3
([#3454](#3454))
([a112559](a112559))
* update google.cloud.opentelemetry.version to v0.33.0
([#3436](#3436))
([e519626](e519626))
* update googleapis/java-cloud-bom digest to 93740d5
([#3280](#3280))
([f987db5](f987db5))
* update grpc dependencies to v1.68.2
([#3420](#3420))
([6d0a169](6d0a169))
* update grpc dependencies to v1.69.0
([#3451](#3451))
([0effb37](0effb37))
* update junit5 monorepo to v5.11.3
([#3310](#3310))
([4e33f2b](4e33f2b))
* update netty dependencies
([#3423](#3423))
([22ea7b5](22ea7b5))
* update opentelemetry-java monorepo to v1.44.1
([#3437](#3437))
([66b85da](66b85da))
* update opentelemetry-java monorepo to v1.45.0
([#3439](#3439))
([d29475a](d29475a))
* update repo-automation-bots digest to 6662ddc
([#3421](#3421))
([dec5de0](dec5de0))
* update repo-automation-bots digest to 8230b4e
([#3448](#3448))
([e53199b](e53199b))
* update repo-automation-bots digest to ae0a72f
([#3452](#3452))
([331bacf](331bacf))
* update repo-automation-bots digest to f3fbbab
([#3419](#3419))
([343a029](343a029))


### Documentation

* explain default generation config in `README.md`
([#3388](#3388))
([2ac86de](2ac86de))
* hermetic build README syntax fix
([#3391](#3391))
([cf32e72](cf32e72))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blakeli0 blakeli0 blakeli0 approved these changes

@diegomarquezp diegomarquezp Awaiting requested review from diegomarquezp

Assignees
No one assigned
Labels
size: s Pull request size is small.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Library name incorrectly interpreted in library generation workflow
2 participants
@JoeWang1127 @blakeli0