FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Automatic-Module-Name added to the manifest. by TheIndifferent · Pull Request #69 · googleapis/api-common-java · GitHub

This repository was archived by the owner on Sep 27, 2023. It is now read-only.
/ api-common-java Public archive

Automatic-Module-Name added to the manifest. - #69

Merged
sduskis merged 2 commits into
googleapis:masterfrom
TheIndifferent:automatic-module-name
Feb 11, 2019
Merged

sduskis merged 2 commits into
googleapis:masterfrom
TheIndifferent:automatic-module-name

Conversation

Copy link
Copy Markdown
Contributor

As described in http://branchandbound.net/blog/java/2017/12/automatic-module-name/ , adding Automatic-Module-Name allows us to move to Java 11 without worrying that the jar file name will change.

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Dec 15, 2018

codecov-io commented Dec 15, 2018
edited by codecov Bot
Loading

Copy link
Copy Markdown

Codecov Report

Merging #69 into master will increase coverage by 0.19%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master      #69      +/-   ##
============================================
+ Coverage     60.19%   60.38%   +0.19%     
- Complexity      138      143       +5     
============================================
  Files            14       14              
  Lines           603      616      +13     
  Branches         92       94       +2     
============================================
+ Hits            363      372       +9     
- Misses          217      219       +2     
- Partials         23       25       +2
Impacted Files Coverage Δ Complexity Δ
...java/com/google/api/pathtemplate/PathTemplate.java 64.85% <0%> (+0.16%) 88% <0%> (+5%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e6bd3cf...429d95d. Read the comment docs.

sduskis requested review from a team and michaelbausor January 14, 2019 13:13

Copy link
Copy Markdown
Contributor

Re module name, what about com.google.api.api-common? I think that just com.google.api.common may not be unique enough (e.g. what about code (either inside or outside Google) placed in a package named com.google.api.common?)

Copy link
Copy Markdown
Contributor Author

@michaelbausor - is a forbidden symbol in JPMS module names. Seems like mostly just the top level package is used, in this case there are multiple packages so hard to choose one. Maybe com.google.api.core?

Copy link
Copy Markdown
Contributor

@TheIndifferent good point re -, sorry about that.

I found reading this comment very useful, although I still don't know what the right answer is: google/guava#2846 (comment)

Basically, I agree with the structure of the name that you proposed at first, which was <common-package>.<some-name-for-this-package>, but I was worried that common might not be a unique enough name.

I would worry that it might be confusing to use com.google.api.core if that covers only some of the package names in the repo - but, I really am not very confident.

Is something like com.google.api.apicommon just too weird/unreadable?

Copy link
Copy Markdown
Contributor Author

@michaelbausor no actually com.google.api.apicommon is descriptive enough. Should I update the PR with that one?

Copy link
Copy Markdown
Contributor

@TheIndifferent I'm happy with that if you are. Thanks!

yoshi-automation added the 🚨 This issue needs some love. label Feb 7, 2019

sduskis commented Feb 10, 2019

Copy link
Copy Markdown
Contributor

@michaelbausor, I updated the PR as per your comments. Can you please review?

sduskis merged commit 0f35062 into googleapis:master Feb 11, 2019
yoshi-automation removed the 🚨 This issue needs some love. label Apr 6, 2020
miraleung pushed a commit that referenced this pull request Jun 4, 2020
* Automatic-Module-Name added to the manifest.

* Update build.gradle
This file contains hidden or 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
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants


Back | FazBrowse Home | New Git URL