Skip to content

initial code removal (limited number of apparently unused code) #858

Merged
tgregg merged 2 commits intoamazon-ion:masterfrom
carljmosca:remove-unused-code
May 16, 2024
Merged

initial code removal (limited number of apparently unused code) #858
tgregg merged 2 commits intoamazon-ion:masterfrom
carljmosca:remove-unused-code

Conversation

@carljmosca
Copy link
Contributor

Issue #, if available:
#763
Description of changes:
Removed unused IonTextBufferedStream class
Removed unused boolean parameter has_sign from loadRadixValue method of IonReaderTextRawTokensX class
Removed unused private method read_utf8 from UnifiedInputStreamX class

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

Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

Thanks @carljmosca, this looks good to me. One request: could you please run ./gradlew :spotlessApply and commit the resulting changes to your branch? We're migrating to a new copyright header, and any files that are touched and don't yet have the new header will cause the build to fail. Once that's done we'll merge this.

@carljmosca
Copy link
Contributor Author

Happy to, thank you @tgregg - I appreciate your feedback and understanding. I should probably know this but should the spotless plugin be updated to the latest release? I don't have it in front of me but I think I saw another issue with the current release. TIA Carl

@tgregg
Copy link
Contributor

tgregg commented May 16, 2024

It's been working for me as-is, but let me know if you run into issues with it.

@carljmosca
Copy link
Contributor Author

carljmosca commented May 16, 2024

If I move the spotless plugin from 6.11.0 to 6.25.0, the imports are corrected and the copyright is updated. Moving back to 6.11.0 (w/o changing source files) returns a java.lang.reflect.InvocationTargetException which I have not yet put time into researching.

@tgregg tgregg force-pushed the remove-unused-code branch from 8ec7ffc to 8771b24 Compare May 16, 2024 19:53
Copy link
Contributor

@tgregg tgregg left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

@tgregg tgregg merged commit 13ece84 into amazon-ion:master May 16, 2024
@carljmosca
Copy link
Contributor Author

Thank you @tgregg - never know with the first one but you certainly made it easy...more coming.

linlin-s pushed a commit that referenced this pull request Jul 3, 2024
* initial code removal

* imports/copyright updates via spotless apply
linlin-s pushed a commit that referenced this pull request Jul 3, 2024
* initial code removal

* imports/copyright updates via spotless apply
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.

2 participants