Skip to content

Conversation

atakavci
Copy link
Collaborator

No description provided.

@atakavci atakavci requested review from ggivo and tishun and removed request for ggivo March 10, 2025 08:16
Copy link

@tishun tishun left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@ggivo ggivo left a comment

Choose a reason for hiding this comment

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

Regarding formatting, I think we should consider using a more recent version of hbase_formatter.xml. Current is versioned 12 from 2019, and later is from 2022 versioned 22. Should support some more up to date IDE settings, but basically the same.

https://github.com/apache/hbase/commits/master/dev-support/hbase_eclipse_formatter.xml

<profiles version="22">
    <profile kind="CodeFormatterProfile" name="'HBase Apache Current'" version="22">

@atakavci
Copy link
Collaborator Author

atakavci commented Mar 10, 2025

hey @ggivo , did you experience this version 22 on any code base ??
asking this because though i tried to re-apply a couple of fixes that i did on version 12, i got things like below with version 22.

    public IAuthenticationResult requestWithConfidentialClient(ConfidentialClientApplication app,
        ClientCredentialParameters params) {
        try {
           ...
    public void testTokenManagerWithHangingTokenRequest()
        throws InterruptedException, ExecutionException, TimeoutException {
        int delayDuration = 200;
        ...

you see line 2 and line 3 are aligned in these, which i dont like.

@atakavci
Copy link
Collaborator Author

besides with a quick look seeing something like this parentheses_positions_in_method_delcaration in source, kind of emitting bad signals in terms of review&test.

@ggivo
Copy link
Contributor

ggivo commented Mar 11, 2025

hey @ggivo , did you experience this version 22 on any code base ?? asking this because though i tried to re-apply a couple of fixes that i did on version 12, i got things like below with version 22.

No, I have not yet. I just saw that it is the version used on the HBase code base. I was going to experiment with it for Jedis formatting.

@atakavci
Copy link
Collaborator Author

i think i will go merge this PR with old version of hbase if you feel comfortable. This repo is less of a subject to discuss on formatting, relative to Jedis.

@atakavci atakavci merged commit 8ffc9c8 into main Mar 13, 2025
3 checks passed
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