Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Format for Java 17 instead of Java 5 #20185

Merged
merged 3 commits into from
Nov 14, 2024
Merged

chore: Format for Java 17 instead of Java 5 #20185

merged 3 commits into from
Nov 14, 2024

Conversation

Artur-
Copy link
Member

@Artur- Artur- commented Oct 8, 2024

No description provided.

Copy link

github-actions bot commented Oct 8, 2024

Test Results

1 146 files  ± 0  1 146 suites  ±0   1h 32m 53s ⏱️ -40s
7 489 tests ± 0  7 439 ✅ ± 0  50 💤 ±0  0 ❌ ±0 
7 811 runs   - 48  7 753 ✅  - 46  58 💤  - 2  0 ❌ ±0 

Results for commit 09e5d21. ± Comparison against base commit 05f0377.

♻️ This comment has been updated with latest results.

@Artur- Artur- requested a review from tepi October 8, 2024 15:33
@Artur-
Copy link
Member Author

Artur- commented Nov 13, 2024

Is there something unclear with this? Planning to go back to Java 5? :)

@tepi
Copy link
Contributor

tepi commented Nov 13, 2024

Is there something unclear with this? Planning to go back to Java 5? :)

No plans currently but we can consider if it's requested.

There's some weird formatting in this PR like https://github.com/vaadin/flow/pull/20185/files#diff-47044a537fba8b2a6975f168bad9c93d53b03670a2337d51732880433b5dfbdeR77-R80

Also https://github.com/vaadin/flow/pull/20185/files#diff-c45aec1725063f2febff465083954b8eca10fa2b4e2de26a92ba949d57dfac5cR88-R93 is probably correct but previous version looked more clear, maybe it was not formatted before this PR?

Edit: fixed the first one. Maybe the second one can be left as is.

@Artur-
Copy link
Member Author

Artur- commented Nov 14, 2024

Yeah it's not clear why it changes the formatting in the second case you mentioned. The thing it is able to handle when set to Java 17 is records formatting, that is completely broken with Java 5

Copy link

sonarcloud bot commented Nov 14, 2024

@tepi tepi merged commit 5891c2c into main Nov 14, 2024
26 checks passed
@tepi tepi deleted the format-java17 branch November 14, 2024 07:23
@vaadin-bot
Copy link
Collaborator

Hi @Artur- and @tepi, when i performed cherry-pick to this commit to 24.5, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 5891c2c
error: could not apply 5891c2c... chore: Format for Java 17 instead of Java 5 (#20185)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

@vaadin-bot
Copy link
Collaborator

Hi @Artur- and @tepi, when i performed cherry-pick to this commit to 24.4, i have encountered the following issue. Can you take a look and pick it manually?
Error Message:
Error: Command failed: git cherry-pick 5891c2c
error: could not apply 5891c2c... chore: Format for Java 17 instead of Java 5 (#20185)
hint: After resolving the conflicts, mark them with
hint: "git add/rm ", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

tepi added a commit that referenced this pull request Nov 14, 2024
* chore: Format for Java 17 instead of Java 5 (#20185) (CP: 24.4)

* format HasValidator.java

---------

Co-authored-by: Teppo Kurki <[email protected]>
tepi added a commit that referenced this pull request Nov 14, 2024
* chore: Format for Java 17 instead of Java 5 (#20185) (CP: 24.5)

* format HasValidator.java

---------

Co-authored-by: Teppo Kurki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants