Skip to content

Refactor ion-java-cli to standard layout#545

Merged
jobarr-amzn merged 1 commit intoamazon-ion:masterfrom
gliptak:gradle1
Aug 25, 2023
Merged

Refactor ion-java-cli to standard layout#545
jobarr-amzn merged 1 commit intoamazon-ion:masterfrom
gliptak:gradle1

Conversation

@gliptak
Copy link
Copy Markdown
Contributor

@gliptak gliptak commented Aug 4, 2023

Issue #, if available: fix (partial) #66

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (b9e5fe6) 66.88% compared to head (d630c5b) 66.88%.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #545   +/-   ##
=========================================
  Coverage     66.88%   66.88%           
+ Complexity     5412     5411    -1     
=========================================
  Files           156      156           
  Lines         22769    22769           
  Branches       4095     4095           
=========================================
+ Hits          15228    15230    +2     
+ Misses         6246     6243    -3     
- Partials       1295     1296    +1     

see 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gliptak
Copy link
Copy Markdown
Contributor Author

gliptak commented Aug 10, 2023

@jobarr-amzn please review

Copy link
Copy Markdown
Contributor

@popematt popematt left a comment

Choose a reason for hiding this comment

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

I took a look at this. You're right that the current project layout does not follow the Gradle convention. However, the downside of relocating these files is that it becomes harder to trace their history in GitHub.

Is there any particular reason you want to make this change, aside from it being more conventional?

@gliptak
Copy link
Copy Markdown
Contributor Author

gliptak commented Aug 23, 2023

yes, following convention (and improving tooling friendliness)

as for diff, git offers support https://stackoverflow.com/q/5730460

@popematt
Copy link
Copy Markdown
Contributor

git offers support

Yes, I am aware of that. I was actually more interested in whether GitHub would correctly show the "blame" on moved/renamed files—it's a helpful tool sometimes to find out when and why a particular bit of code was introduced (and changed over time). That being said, I went and tested it out on the PR branch just to be sure, and to my surprise, the blame is preserved. (I wonder when that started happening... or maybe I'm getting confused with a different git host.)

Anyway, with that out of the way, I see no reason not to accept this change.

Signed-off-by: Gábor Lipták <gliptak@gmail.com>
@jobarr-amzn
Copy link
Copy Markdown
Contributor

Apologies, @gliptak - I was wrapped up in some other work the past couple of weeks and this one slipped past me. Thanks for reviewing it, @popematt!

@jobarr-amzn jobarr-amzn merged commit 0b2c363 into amazon-ion:master Aug 25, 2023
@gliptak gliptak deleted the gradle1 branch August 26, 2023 15:34
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