Bazel API improvements #8985
Merged
Bazel API improvements #8985
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sschuberth
force-pushed
the
bazel-api-imps
branch
2 times, most recently
from
c5392d5 to
b9bc575
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8985 +/- ##
============================================
+ Coverage 67.97% 68.02% +0.05%
Complexity 1158 1158
============================================
Files 241 241
Lines 7690 7690
Branches 867 867
============================================
+ Hits 5227 5231 +4
+ Misses 2106 2103 -3
+ Partials 357 356 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
sschuberth
force-pushed
the
bazel-api-imps
branch
2 times, most recently
from
9cf85a7 to
3d162ac
Compare
nnobelis
reviewed
Aug 14, 2024
| @@ -47,7 +47,9 @@ internal class CompositeBazelModuleRegistryService( | |||
| * created based on the passed in [urls]; local registries use the given [projectDir] as workspace. | |||
| */ | |||
| fun create(urls: Collection<String>, projectDir: File): CompositeBazelModuleRegistryService { | |||
| val packageNamesForServer = urls.filter { it.endsWith("source.json") }.mapNotNull { url -> | |||
| val registryUrls = urls.distinctBy { it.removeSuffix("/") } | |||
There was a problem hiding this comment.
I don't understand why you need disambiguation here when there is the filter underneath that accepts only URLs ending with source.json.
There was a problem hiding this comment.
Well, couldn't there still be the same URL ending with "source.json" multiple times in the collection? But anyway I agree that the removeSuffix("/") "normalization" can be omitted here.
mnonnenmacher
requested changes
Aug 14, 2024
clients/bazel-module-registry/src/main/kotlin/LocalBazelModuleRegistryService.kt
Outdated
Show resolved
Hide resolved
The only place where the function is called with theoretically nullable arguments is from within a mock in a test, but the compiler seems to accept this. Signed-off-by: Sebastian Schuberth <[email protected]>
Be concrete when producing lists and lenient when accepting collections. Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
Signed-off-by: Sebastian Schuberth <[email protected]>
This avoids output like
Creating remote Bazel module registry at 'https://bcr.bazel.build/'.
Creating remote Bazel module registry at 'https://bcr.bazel.build'.
Signed-off-by: Sebastian Schuberth <[email protected]>
sschuberth
force-pushed
the
bazel-api-imps
branch
from
3d162ac to
437137d
Compare
mnonnenmacher
approved these changes
Aug 15, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Please have a look at the individual commit messages for the details.