Skip to content

Conversation

@weihubeats
Copy link
Member

@weihubeats weihubeats commented Aug 1, 2025

Purpose of the pull request

#465

What's changed?

  1. Fix the incorrectly printed log level
  2. Add a generic function to reduce excessive if statements

Checklist

  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Copy link
Member

@alaahong alaahong left a comment

Choose a reason for hiding this comment

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

Seems not mandatory to create a new Class with new Method?
e.g. inline usage is enough

 Optional.ofNullable(writeCellStyle.getHidden())
        .ifPresent(cellStyle::setHidden); 

And meanwhile, even for new Utils, please also provide unit test at same time.

@weihubeats
Copy link
Member Author

We can delete the new utility classes and use internal private methods.

@alaahong
Copy link
Member

alaahong commented Aug 3, 2025

[x] I have added the necessary unit tests and all cases have passed.
Please also fulfill this as you've added several methods~

@psxjoy
Copy link
Member

psxjoy commented Aug 5, 2025

All CI fails

@weihubeats
Copy link
Member Author

fix ci

Copy link
Member

@psxjoy psxjoy left a comment

Choose a reason for hiding this comment

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

Something needs to change.

@weihubeats
Copy link
Member Author

What else needs to be done

Copy link
Member

@alaahong alaahong left a comment

Choose a reason for hiding this comment

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

why involve new version declaration

@psxjoy psxjoy self-requested a review August 7, 2025 13:46
Copy link
Member

@psxjoy psxjoy left a comment

Choose a reason for hiding this comment

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

LGTM
If no one has other questions, this PR will merge in 24h.

Copy link
Member

@tisonkun tisonkun left a comment

Choose a reason for hiding this comment

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

LGTM.

Thanks for your contribution!

Merging ...

@tisonkun tisonkun merged commit 2bf3402 into apache:main Aug 11, 2025
5 checks passed
@delei delei added this to the 1.3.0 milestone Aug 11, 2025
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.

5 participants