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

[Summary] Enhance code style warnings in server #2435

Closed
16 of 18 tasks
msgui opened this issue Jan 31, 2024 · 8 comments
Closed
16 of 18 tasks

[Summary] Enhance code style warnings in server #2435

msgui opened this issue Jan 31, 2024 · 8 comments
Labels
ci-cd Build or deploy good first issue Good for newcomers improvement General improvement
Milestone

Comments

@msgui
Copy link
Contributor

msgui commented Jan 31, 2024

SubTask

PS: One PR for multiple modules is fine, but if the total code exceeds 4000 lines, split it into multiple PRs.

Expected & Actual behavior (期望与实际表现)

image
image
image

TODO

  • IDEA formatting plugin currently lacks support for adding a blank line after enum declarations.image
  • Use spotless to format code
@msgui msgui added the bug Something isn't working label Jan 31, 2024
Copy link

dosubot bot commented Jan 31, 2024

🤖

I'm sorry, but I'm unable to view the images you've attached. However, I can provide some general advice on why the server module might be triggering warnings when using check-style.

The server module might trigger warnings due to several reasons:

  1. Line Length: The recommended line length is less than or equal to 100 characters. If the code in the server module exceeds this limit, it will trigger a warning.

  2. Alignment and Indentation: The code style guide suggests that for strings and function parameters, alignment has a higher priority than indentation. If the code in the server module doesn't follow this rule, it will trigger a warning.

  3. Empty Lines: The code style guide specifies certain situations where an empty line should be added. If the code in the server module doesn't follow these rules, it will trigger a warning.

  4. Variable and Method Naming: The code style guide suggests using camel case for variable and method naming, and method names should not have get, set, is prefixes. If the code in the server module doesn't follow these rules, it will trigger a warning.

  5. Error Checking: The code style guide suggests using E.checkArgument() and E.checkState() for error checking instead of if + throw. If the code in the server module doesn't follow this rule, it will trigger a warning.

  6. Exception Handling: The code style guide suggests handling exceptions at the top-level call or external API call. If the code in the server module doesn't follow this rule, it will trigger a warning.

  7. Logging: The code style guide suggests using {} for placeholders in log messages instead of string concatenation. If the code in the server module doesn't follow this rule, it will trigger a warning.

  8. String Concatenation: The code style guide suggests using String.format() for string concatenation. If the code in the server module doesn't follow this rule, it will trigger a warning.

Please check the server module code against these rules to identify the specific cause of the warnings. You can refer to the Hugegraph Code Style Guide for more details.

Without the specific warnings or the exact lines of code that are causing these warnings, it's difficult to provide a more precise answer. If you could provide more details about the warnings, such as the warning messages or the lines of code that are causing these warnings, I would be able to provide a more accurate response.

Sources


This response is meant to be useful and save you time. It is not meant to be a precise solution, but rather a starting point for your own research.

Help me be more useful! Please leave a 👍 if this is helpful and 👎 if it is irrelevant.

If you want to continue the conversation, start your reply with @dosu-bot.

@dosubot dosubot bot added the ci-cd Build or deploy label Jan 31, 2024
@msgui msgui changed the title [Bug] Checkstyle warnings in server [Bug] Compile warnings in server Jan 31, 2024
@imbajin imbajin added improvement General improvement good first issue Good for newcomers and removed bug Something isn't working labels Jan 31, 2024
@imbajin imbajin changed the title [Bug] Compile warnings in server [Enhancement] Fix check-style compile warnings in server Jan 31, 2024
@VGalaxies
Copy link
Contributor

@msgui consider introducing the specific steps for formatting, cleaning, and correcting in the relevant PRs to help us in PR validation :)

@imbajin imbajin changed the title [Enhancement] Fix check-style compile warnings in server [Summary] Enhance code style warnings in server Feb 2, 2024
@imbajin imbajin modified the milestones: 1.3.0, 1.5.0 Feb 2, 2024
@imbajin imbajin moved this to 🏗 In progress in HugeGraph Tasks Feb 2, 2024
@msgui
Copy link
Contributor Author

msgui commented Feb 3, 2024

consider introducing the specific steps for formatting, cleaning, and correcting in the relevant PRs to help us in PR validation :)

  1. Configure IDEA based on the Config Style full version - fs.

  2. Review whether hugegraph-style.xml aligns with the specifications outlined in the HugeGraph Code Style Guide.

  3. Import the latest license header (found at the end of the HugeGraph Code Style Guide) into IDEA and utilize the "Update Copyright" feature to keep the license header up-to-date.

  4. Utilize IDEA plugins such as Reformat and Clean Code for code formatting.

imbajin added a commit that referenced this issue Feb 7, 2024
subtask of #2435

---------

Co-authored-by: imbajin <jin@apache.org>
Co-authored-by: VGalaxies <vgalaxies@apache.org>
VGalaxies added a commit that referenced this issue Feb 7, 2024
subtask of #2435

---------

Co-authored-by: imbajin <jin@apache.org>
Co-authored-by: VGalaxies <vgalaxies@apache.org>
returnToInnocence added a commit to msgui/incubator-hugegraph that referenced this issue Feb 21, 2024
@returnToInnocence
Copy link
Contributor

Adhere to the code conventions outlined in HugeGraph-Code-Style-Guide (please refine the hg-style.xml specification @returnToInnocence)#2457

@returnToInnocence
Copy link
Contributor

returnToInnocence commented Feb 27, 2024

Suddenly a new idea, perhaps as a future thing to do, is to try to migrate the entire contents of the entire hg-style.xml file to.editorconfig

This is because the.editorconfig file has some other advantages that hg-style.xml does not have when I was looking at the documentation for this, such as:

  • The.editorconfig file can also define coding conventions that cannot be defined by hg-style.xml, such as code style, formatting, and naming, which can help improve code consistency and readability.
  • The.editorconfig file does not require the user to make any additional configuration, such as importing code style. If this file exists in the project, it will be automatically invoked by default, and code sytle will be overwritten

Therefore, I wonder if we should further try to gather the configuration of a function as much as possible in the future, instead of spreading it into multiple Settings, and reduce the user's perception of format as little as possible at one time

@VGalaxies
Copy link
Contributor

VGalaxies commented Feb 27, 2024

Suddenly a new idea, perhaps as a future thing to do, is to try to migrate the entire contents of the entire hg-style.xml file to.editorconfig

This is because the.editorconfig file has some other advantages that hg-style.xml does not have when I was looking at the documentation for this, such as:

  • The.editorconfig file can also define coding conventions that cannot be defined by hg-style.xml, such as code style, formatting, and naming, which can help improve code consistency and readability.
  • The.editorconfig file does not require the user to make any additional configuration, such as importing code style. If this file exists in the project, it will be automatically invoked by default, and code sytle will be overwritten

Therefore, I wonder if we should further try to gather the configuration of a function as much as possible in the future, instead of spreading it into multiple Settings, and reduce the user's perception of format as little as possible at one time

Indeed, I found that #2457 uses both hugegraph-style.xml and .editorconfig to control code style. If the control scope of the editorconfig file is broader than the idea code style XML, it might be worth considering consolidating them into .editorconfig.

In the context of IntelliJ IDEA, perhaps this is feasible, see https://www.jetbrains.com/help/idea/editorconfig.html

image

Perhaps it's possible to export the current hugegraph-style.xml to .editorconfig format and merge it with the existing .editorconfig to see if the previously defined styles are compromised...

@returnToInnocence
Copy link
Contributor

returnToInnocence commented Feb 27, 2024

Suddenly a new idea, perhaps as a future thing to do, is to try to migrate the entire contents of the entire hg-style.xml file to.editorconfig
This is because the.editorconfig file has some other advantages that hg-style.xml does not have when I was looking at the documentation for this, such as:

  • The.editorconfig file can also define coding conventions that cannot be defined by hg-style.xml, such as code style, formatting, and naming, which can help improve code consistency and readability.
  • The.editorconfig file does not require the user to make any additional configuration, such as importing code style. If this file exists in the project, it will be automatically invoked by default, and code sytle will be overwritten

Therefore, I wonder if we should further try to gather the configuration of a function as much as possible in the future, instead of spreading it into multiple Settings, and reduce the user's perception of format as little as possible at one time

Indeed, I found that #2457 uses both hugegraph-style.xml and .editorconfig to control code style. If the control scope of the editorconfig file is broader than the idea code style XML, it might be worth considering consolidating them into .editorconfig.

In the context of IntelliJ IDEA, perhaps this is feasible, see https://www.jetbrains.com/help/idea/editorconfig.html
image

Perhaps it's possible to export the current hugegraph-style.xml to .editorconfig format and merge it with the existing .editorconfig to see if the previously defined styles are compromised...

get it! I will try to export it this afternoon to see how it works!
image

@returnToInnocence
Copy link
Contributor

returnToInnocence commented Feb 28, 2024

Since the above two tasks are logically coherent, I merged them into one @msgui
https://hugegraph.feishu.cn/wiki/NzaxwWIRHi5mJGkXkR0cHx6mnbg

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-cd Build or deploy good first issue Good for newcomers improvement General improvement
Projects
Status: Done
Development

No branches or pull requests

4 participants