From 09287085e84a3aea02b93d1440d039d3fdba70ee Mon Sep 17 00:00:00 2001 From: yangsngshaoxue Date: Thu, 25 Aug 2022 16:32:36 +0800 Subject: [PATCH] docs: Add configuration and Update contributor Add configuration and Update contributor Change-Id: I76fb6d32108d2e36a96d877835441cb888a37673 --- doc/source/configuration/index.rst | 2 + doc/source/configuration/settings.rst | 50 ++++++++ doc/source/contributor/contributing.rst | 1 - doc/source/contributor/gerrit.rst | 162 ++++++++++++++++++++++++ doc/source/contributor/releasenotes.rst | 101 +++++++++++++++ 5 files changed, 315 insertions(+), 1 deletion(-) create mode 100644 doc/source/configuration/settings.rst diff --git a/doc/source/configuration/index.rst b/doc/source/configuration/index.rst index fd8324f0..be43c8fb 100644 --- a/doc/source/configuration/index.rst +++ b/doc/source/configuration/index.rst @@ -4,3 +4,5 @@ Configuration Guide .. toctree:: :maxdepth: 1 + + settings diff --git a/doc/source/configuration/settings.rst b/doc/source/configuration/settings.rst new file mode 100644 index 00000000..ee2e046c --- /dev/null +++ b/doc/source/configuration/settings.rst @@ -0,0 +1,50 @@ +.. _configuration-settings: + +================== +Settings Reference +================== + +- Prepare a usable backend + - Prepare an accessible backend, for example: `https://172.20.154.250` + - Modify the corresponding configuration in `config/webpack.dev.js`: + + .. code:: javascript + + if (API === 'mock' || API === 'dev') { + devServer.proxy = { + '/api': { + target: 'https://172.20.154.250', + changeOrigin: true, + secure: false, + }, + }; + } + +- Configure access host and port + - Modify `devServer.host` and `devServer.port` + - Modify the corresponding configuration in `config/webpack.dev.js` + + .. code:: javascript + + const devServer = { + host: '0.0.0.0', + // host: 'localhost', + port: 8088, + contentBase: root('dist'), + historyApiFallback: true, + compress: true, + hot: true, + inline: true, + disableHostCheck: true, + // progress: true + }; + +- Execute in the project root directory, which is the same level as `package.json` + + .. code:: shell + + yarn run dev + + +- Use the `host` and `port` configured in `config/webpack.dev.js` to access, such as `http://localhost:8088` +- The front-end real-time update environment used for development is done. diff --git a/doc/source/contributor/contributing.rst b/doc/source/contributor/contributing.rst index 44098d24..3797ec00 100644 --- a/doc/source/contributor/contributing.rst +++ b/doc/source/contributor/contributing.rst @@ -35,7 +35,6 @@ IRC answered: http://eavesdrop.openstack.org/irclogs/%23openstack-skyline/ weekly meeting - .. note:: Now we have not weekly meeting, we will have it in the future. diff --git a/doc/source/contributor/gerrit.rst b/doc/source/contributor/gerrit.rst index 2f1a95f7..094be606 100644 --- a/doc/source/contributor/gerrit.rst +++ b/doc/source/contributor/gerrit.rst @@ -2,3 +2,165 @@ Code Reviews ============ + +Skyline Console follows the same `Review guidelines`_ outlined by the +OpenStack community. This page provides additional information that is +helpful for reviewers of patches to Skyline Console. + +Gerrit +------ + +Skyline Console uses the `Gerrit`_ tool to review proposed code changes. +The review site is https://review.opendev.org + +Gerrit is a complete replacement for Github pull requests. `All Github pull +requests to the Skyline Console repository will be ignored`. + +See `Quick Reference`_ for information on quick reference for developers. +See `Getting Started`_ for information on how to get started using Gerrit. +See `Development Workflow`_ for more detailed information on how to work with +Gerrit. + +The Great Change +---------------- + +Skyline Console has a modern technology stack and ecology, is easier for +developers to maintain and operate by users, and has higher concurrency +performance. And it focus on functional design and user experience. Embrace +modern browser technology and ecology: React, Ant Design and Mobx. Use React +component to process rendering, the page display process is fast and smooth, +bringing users a better UI and UE experience. + +Unit Tests +---------- + +Skyline Console requires unit tests with all patches that introduce a new +branch or function in the code. Changes that do not come with a +unit test change should be considered closely and usually returned +to the submitter with a request for the addition of unit test. + +CI Job rechecks +--------------- + +CI job runs may result in false negatives for a considerable number of causes: + +- Network failures. +- Not enough resources on the job runner. +- Storage timeouts caused by the array running nightly maintenance jobs. +- External service failure: pypi, package repositories, etc. +- Non skyline-console components spurious bugs. + +And the list goes on and on. + +When we detect one of these cases the normal procedure is to run a recheck +writing a comment with ``recheck`` for core Zuul jobs. + +These false negative have periods of time where they spike, for example when +there are spurious failures, and a lot of rechecks are necessary until a valid +result is posted by the CI job. And it's in these periods of time where people +acquire the tendency to blindly issue rechecks without looking at the errors +reported by the jobs. + +When these blind checks happen on real patch failures or with external services +that are going to be out for a while, they lead to wasted resources as well as +longer result times for patches in other projects. + +The Skyline community has noticed this tendency and wants to fix it, so now +it is strongly encouraged to avoid issuing naked rechecks and instead issue +them with additional information to indicate that we have looked at the failure +and confirmed it is unrelated to the patch. + +Efficient Review Guidelines +--------------------------- + +This section will guide you through the best practices you can follow to do +quality code reviews: + +* **Failing Gate**: You can look for possible failures in linting, unit test, + functional test etc and provide feedback on fixing it. Usually it's the + author's responsibility to do a local run of tox and ensure they don't + fail upstream but if something is failing on gate and the author is not + be aware about how to fix it then we can provide valuable guidance on it. + +* **Documentation**: Check whether the patch proposed requires documentation + or not and ensure the proper documentation is added. If the proper + documentation is added then the next step is to check the status of docs job + if it's failing or passing. If it passes, you can check how it looks in HTML + as follows: + Go to ``openstack-tox-docs job`` link -> ``View Log`` -> ``docs`` and go to + the appropriate section for which the documentation is added. + Rendering: We do have a job for checking failures related to document + changes proposed (openstack-tox-docs) but we need to be aware that even if + a document change passes all the syntactical rules, it still might not be + logically correct i.e. after rendering it could be possible that the bullet + points are not under the desired section or the spacing and indentation is + not as desired. It is always good to check the final document after rendering + in the docs job which might yield possible logical errors. + +* **Readability**: Readability is a big factor as remembering the logic of + every code path is not feasible and contributors change from time to time. + We should adapt to writing readable code which is easy to follow and can be + understood by anyone having knowledge about JavaScript and working of + Skyline Console. Sometimes it happens that a logic can only be written in + a complex way, in that case, it's always good practice to add a comment + describing the functionality. So, if a logic proposed is not readable, do + ask/suggest a more readable version of it and if that's not feasible then + asking for a comment that would explain it is also a valid review point. + +* **Downvoting reason**: It often happens that the reviewer adds a bunch of + comments some of which they would like to be addressed (blocking) and some + of them are good to have but not a hard requirement (non-blocking). It's a + good practice for the reviewer to mention for which comments is the -1 valid + so to make sure they are always addressed. + +* **Testing**: Always check if the patch adds the associated unit, functional + and e2e tests depending on the change. + +* **Commit Message**: There are few things that we should make sure the commit + message includes: + + 1) Make sure the author clearly explains in the commit message why the + code changes are necessary and how exactly the code changes fix the + issue. + + 2) It should have the appropriate tags (Eg: Closes-Bug, Related-Bug, + Blueprint, Depends-On etc). For detailed information refer to + `external references in commit message`_. + + 3) It should follow the guidelines of commit message length i.e. + 50 characters for the summary line and 72 characters for the description. + More information can be found at `Summary of Git commit message structure`_. + + 4) Sometimes it happens that the author updates the code but forgets to + update the commit message leaving the commit describing the old changes. + Verify that the commit message is updated as per code changes. + +* **Release Notes**: There are different cases where a releasenote is required + like fixing a bug, adding a feature, changing areas affecting upgrade etc. + You can refer to the `Release notes`_ section in our contributor docs for + more information. + +* **Ways of reviewing**: There are various ways you can go about reviewing a + patch, following are some of the standard ways you can follow to provide + valuable feedback on the patch: + + 1) Testing it in local environment: The easiest way to check the correctness + of a code change proposed is to reproduce the issue (steps should be in + launchpad bug) and try the same steps after applying the patch to your + environment and see if the provided code changes fix the issue. + You can also go a little further to think of possible corner cases where an + end user might possibly face issues again and provide the same feedback to + cover those cases in the original change proposed. + + 2) Optimization: If you're not aware about the code path the patch is fixing, + you can still go ahead and provide valuable feedback about the python code + if that can be optimized to improve maintainability or performance. + +.. _Review guidelines: https://docs.openstack.org/doc-contrib-guide/docs-review-guidelines.html +.. _Gerrit: https://review.opendev.org/q/project:openstack/skyline-apiserver+status:open +.. _Quick Reference: https://docs.openstack.org/infra/manual/developers.html#quick-reference +.. _Getting Started: https://docs.openstack.org/infra/manual/developers.html#getting-started +.. _Development Workflow: https://docs.openstack.org/infra/manual/developers.html#development-workflow +.. _external references in commit message: https://wiki.openstack.org/wiki/GitCommitMessages#Including_external_references +.. _Summary of Git commit message structure: https://wiki.openstack.org/wiki/GitCommitMessages#Summary_of_Git_commit_message_structure +.. _Release notes: https://docs.openstack.org/skyline-apiserver/latest/contributor/releasenotes.html diff --git a/doc/source/contributor/releasenotes.rst b/doc/source/contributor/releasenotes.rst index ed195fbd..9fdca50b 100644 --- a/doc/source/contributor/releasenotes.rst +++ b/doc/source/contributor/releasenotes.rst @@ -2,3 +2,104 @@ Release notes ============= + +The release notes for a patch should be included in the patch. + +If the following applies to the patch, a release note is required: + +* Upgrades + + * The deployer needs to take an action when upgrading + * A new config option is added that the deployer should consider changing + from the default + * A configuration option is deprecated or removed + +* Features + + * A new feature is implemented + * Feature is deprecated or removed + * Current behavior is changed + +* Bugs + + * A security bug is fixed + * A long-standing or important bug is fixed + +* APIs + + * REST API changes + + +Reviewing release note content +------------------------------ + +Release notes are user facing. We expect operators to read them (and other +people interested in seeing what's in a new release may read them, too). +This makes a release note different from a commit message, which is aimed +at other developers. + +Keep this in mind as you review a release note. Also, since it's user +facing, something you would think of as a nit in a code comment (for +example, bad punctuation or a misspelled word) is not really a nit in a +release note--it's something that needs to be corrected. This also applies +to the format of the release note, which should follow the standards set +out later in this document. + +In summary, don't feel bad about giving a -1 for a nit in a release note. We +don't want to have to go back and fix typos later, especially for a bugfix +that's likely to be backported, which would require squashing the typo fix into +the backport patch (which is something that's easy to forget). Thus we really +want to get release notes right the first time. + +Fixing a release note +--------------------- + +Of course, even with careful writing and reviewing, a mistake can slip +through that isn't noticed until after a release. If that happens, the +patch to correct a release note must be proposed *directly to the stable branch +in which the release note was introduced*. (Yes, this is completely different +from how we handle bugs.) + +This is because of how reno scans release notes and determines what release +they go with. See `Updating Stable Branch Release Notes +`_ +in the `reno User Guide` for more information. + +Bugs +---- + +For bug fixes, release notes must include the bug number in Launchpad with a +link to it as a RST link. + +Note the use of the past tense ("Fixed") instead of the present tense +("Fix"). This is because although you are fixing the bug right now in the +present, operators will be reading the release notes in the future (at the +time of the release), at which time your bug fix will be a thing of the past. + +Additionally, keep in mind that when your release note is published, it is +mixed in with all the other release notes and won't obviously be connected +to your patch. Thus, in order for it to make sense, you may need to repeat +information that you already have in your commit message. That's OK. + +Creating the note +----------------- + +Skyline Console uses `reno `_ to +generate release notes. Please read the docs for details. In summary, use + +.. code-block:: bash + + $ tox -e venv -- reno new + +Then edit the sample file that was created and push it with your change. + +To see the results: + +.. code-block:: bash + + $ git commit # Commit the change because reno scans git log. + + $ tox -e releasenotes + +Then look at the generated release notes files in releasenotes/build/html in +your favorite browser.