Skip to content

Comments

Upgrade this library to vert.x 5#7

Open
sinri wants to merge 2 commits intocloudonix:mainfrom
sinri:main
Open

Upgrade this library to vert.x 5#7
sinri wants to merge 2 commits intocloudonix:mainfrom
sinri:main

Conversation

@sinri
Copy link

@sinri sinri commented Oct 31, 2025

No logic changed; only changed style from vert.x 4 to vert.x 5.

@guss77
Copy link
Contributor

guss77 commented Oct 31, 2025

You have changed all the indentation from tabs to spaces, which is not OK: in addition to being against the project's original code style, it makes it really hard to review the changes as there are huge blocks that show up as being replaced even though there is likely no change other than tabs and spaces.

I understand that Vert.x internal coding style is to use spaces, but I think they are wrong and I do not feel that its a requirement for all Vert.x related 3rd party projects to use the same indentation style.

If you can update the MR without the unwanted indentation changes, I would appreciate it as it will let me review the changes properly.

@sinri
Copy link
Author

sinri commented Oct 31, 2025

I have refined as I can.
But, I think, the diff about white chars, could be filtered, and could be reformatted.

@guss77
Copy link
Contributor

guss77 commented Oct 31, 2025

There are still a lot of white space changes with no obvious need. Please avoid changing the style of the project.
For example:

  1. Space at the beginning of the line should maintain the indentation of the previous line.
  2. An empty line at a javadoc block should maintain indentation (i.e one space after the asterisk).
  3. Indentation after an lambda opening curly brace is just one tab.
  4. The indentation in the tests is quite broken - after removing the try...catch blocks, the content should be unindented once, and the following lines should match the indentation.

Before committing, please verify that there are no unneeded changes - a change in whitespace when there's no code change on the same line, is unneeded. Please don't try to fix the code style of the entire file when changing code. Lets focus on the code changes.

I also have a content note: You have imported Callabale to use it explicitly in the executeBlocking() call in line 111 of OutputToReadStream, but you did not do so the the same call in WriteToInputStream nor in its test. You shouldn't need to explicitly import and use Callable - so please remove that.

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