RocMarshal commented on code in PR #226: URL: https://github.com/apache/incubator-streampark-website/pull/226#discussion_r1307528367
########## community/submit_guide/pr-and-code-quality.md: ########## @@ -0,0 +1,420 @@ +--- +id: 'pr-and-code-quality.md' +title: 'Pr and code quality' Review Comment: ```suggestion title: 'Code Style and Quality Guide' ``` ########## community/submit_guide/pr-and-code-quality.md: ########## @@ -0,0 +1,420 @@ +--- +id: 'pr-and-code-quality.md' Review Comment: How about renaming file name to `code-style-and-quality-guide.md` ? Please let me know what's your opinion. ########## community/submit_guide/pr-and-code-quality.md: ########## @@ -0,0 +1,420 @@ +--- +id: 'pr-and-code-quality.md' +title: 'Pr and code quality' +sidebar_position: 3 +--- + +<!-- + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + https://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +--> + + +## 1 Pull Requests & Changes Rule + +ISSUE/PR(pull request) driving and naming + +Ensure that PR corresponds to ISSUE. + +Note: Hotfix issue does not need to follow this rule, such as fixing spelling errors in JavaDoc or document files. + +Naming format +When naming PR, you can refer to the [ISSUE-XXXX][Feature/Improve/Character/Cleanup] Title of the pull request, where ISSUE-XXXX should be replaced with the actual ISSUE number. + +These components should be the same as those used in original ISSUEs. +The second part describes the type of PR, such as new features, optimization, refactoring, etc. If all changes to PR are within a certain module or component, they can be indicated in the commit message. +Description + +Please fill in the Pull Request template to describe the contribution. So that the reviewer can understand the problem and solution from the description, rather than just from the code. +Ensure that the description is sufficient to illustrate the problem addressed by the PR. +Small changes do not require too much description. +In an ideal scenario, the problem is described in ISSUE, and most of the description is copied from there. +Try to break down changes into pure types of changes + +It's recommended that Pull requests should be arranged changes such as Cleanup, Refactor, Improve, and Feature into separate PRs/Commits. +In this way, the Reviewers can independently view cleaning and refactoring, and ensure that these changes do not change behavior. +Then, the Reviewer can independently review the core changes and ensure that they are a clean and robust change. +In extreme cases, if a rollback commit is required, it can provide the optimal granularity for version rollback selection. +In addition, significant contributions should be split into a set of independent changes that can be reviewed independently. +Commit message naming +The commit of messages should follow a pattern similar to the PR: [ISSUE-XXXX][Feature/Improve/Refactor/Cleanup] Title of the pull request + +[Hotfix][module_name] Fix xxx comments +[ISSUE-xxxx1][Improvement] Improve +[ISSUE-xxxx2][Refactor] Refactor +[ISSUE-xxxx2][Feature] Support +[ISSUE-xxxx3][Feature][subtask] Support +Note: Try to use git history instead of annotated code (not mandatory) + +## 2 Code Checkstyle + +- Backend code formatting Maven plugin: spotless +Just run `mvn spotless:apply` in the project repo root directory after installing the plugin. + +- Backend code specification Maven plugin: checkstyle +Just run mvn `checkstyle:checkstyle` after installing the plugin. + +- Frontend code formatting plugin eslint + + The original command is `npx eslint --cache --max-warnings 0 "{src,mock}/**/*.{vue,ts,tsx}" --fix` + + Encapsulated as `npm run lint:eslint` + +## 3 Programming Specification + +### 3.1 Naming Style + +1. Prioritize selecting nouns for variable naming + It's easier to distinguish between variables or methods + - Example of variables: + - `Cache<String> publicKeyCache;` + +2. Pinyin abbreviations are prohibited for variables (excluding nouns such as place names)Such as chengdu. + +3. It is recommended to end variable names with a type + For variables of type Collection/List, take xxxx (plural representing multiple elements) or end with xxxList (specific type) + For variables of type map, describe the key and value clearly: + Example: + + - `Map<Long, User> idUserMap` + - `Map<Long, String> userIdNameMap` + +4. That can intuitively know the type and meaning of the variable through its name + Method names should start with a verb first as follows: + + `void computeVcores(Object parameter1);` + +5. The methods name of basic CRUD of the database layer (non-service layer) should be uniformly standardized according to name com.baomidou.mybatisplus.core.mapper.BaseMapper: + + If perform a database select operation, the name of the method should be started with select + + For example, `selectById`, `selectByXxx`, `selectPageByXxx` + + - If perform a database <mark> update </mark> statement operation, the name of the method should be started with `update` + - If perform a database <mark> insert </mark> statement operation, the name of the method should be started with `insert` + - If perform a database <mark> delete </mark> statement operation, the name of the method should be started with `delete` + +6. The methods name of basic CRUD of the service layer should be named as com.baomidou.mybatisplus.extension.service.IService: + + - If perform a database <mark> select </mark> operation to query multiple records, the name of the method should be started with a `list`, such as `listByIds`, `listByXxx` + - If perform a database <mark> select </mark> operation to query a single record, the name of the method should be started with get, such as `getByName` and `getOne` + - If perform a database <mark> update </mark> operation, the name of the method should be started with `update` + - If perform a database <mark> insert </mark> operation, the name of the method should be started with `save` + - If perform a database <mark> delete </mark> operation, the name of the method should be started with `remove` + +### 3.2 Constant Definition + +Set the serialVersionUID of all classes to 1L, following Flink's serialVersionUID +Redundant strings should be modified to constants, here are a positive demo and a negative demo: + +```java Review Comment: I'm very sorry for not being able to maintain a consistent sample style during the draft stage. Could we maintain a consistent format for both positive and negative examples ? - don't: ...... - do: ...... `OR` - negative demo: ..... - positive demo: .... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
