diff --git a/README.rst b/README.rst index 8991d43..0889cb8 100644 --- a/README.rst +++ b/README.rst @@ -15,7 +15,7 @@ to maintain and operate by users, and has higher concurrency performance. You can learn more about Skyline APIServer at: -* `Wiki `__ +* `Wiki `__ * `Developer Docs `__ * `Blueprints `__ * `Release notes `__ diff --git a/doc/source/glossary.rst b/doc/source/common/glossary.rst similarity index 100% rename from doc/source/glossary.rst rename to doc/source/common/glossary.rst diff --git a/doc/source/contributor/contributing.rst b/doc/source/contributor/contributing.rst new file mode 100644 index 0000000..f073086 --- /dev/null +++ b/doc/source/contributor/contributing.rst @@ -0,0 +1,207 @@ +============================ +So You Want to Contribute... +============================ + +For general information on contributing to OpenStack, please check out the +`contributor guide `_ to get started. +It covers all the basics that are common to all OpenStack projects: the +accounts you need, the basics of interacting with our Gerrit review system, how +we communicate as a community, etc. + +Below will cover the more project specific information you need to get started +with the skyline-apiserver project, which is responsible for the following +OpenStack deliverables: + +skyline-apiserver + | The OpenStack Modern Dashboard - back-end. + | code: https://opendev.org/openstack/skyline-apiserver + | docs: https://docs.openstack.org/skyline-apiserver/latest/ + | Launchpad: https://launchpad.net/skyline-apiserver + +Communication +~~~~~~~~~~~~~ + +IRC + We use IRC *a lot*. You will, too. You can find infomation about what + IRC network OpenStack uses for communication (and tips for using IRC) + in the `Setup IRC + `_ + section of the main `OpenStack Contributor Guide`. + + People working on the Skyline APIServer project may be found in the + ``#openstack-skyline`` IRC channel during working hours + in their timezone. The channel is logged, so if you ask a question + when no one is around, you can check the log to see if it's been + answered: http://eavesdrop.openstack.org/irclogs/%23openstack-skyline/ + +weekly meeting + + .. note:: + + Now we have not weekly meeting, we will have it in the future. + +mailing list + We use the openstack-discuss@lists.openstack.org mailing list for + asynchronous discussions or to communicate with other OpenStack teams. + Use the prefix ``[skyline]`` in your subject line (it's a high-volume + list, so most people use email filters). + + More information about the mailing list, including how to subscribe + and read the archives, can be found at: + http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-discuss + +Contacting the Core Team +~~~~~~~~~~~~~~~~~~~~~~~~ + +The skyline-core team is an active group of contributors who are responsible +for directing and maintaining the skyline-apiserver project. As a new +contributor, your interaction with this group will be mostly through code +reviews, because only members of skyline-core can approve a code change to be +merged into the code repository. + +You can learn more about the role of core reviewers in the OpenStack +governance documentation: +https://docs.openstack.org/contributors/common/governance.html#core-reviewer + +The membership list of skyline-core is maintained in gerrit: +https://review.opendev.org/admin/groups/1fe65032c39f1d459327b010730627a904d7b793,members + +Project Team Lead +~~~~~~~~~~~~~~~~~ + +For each development cycle, Skyline APIServer project Active Technical +Contributors (ATCs) elect a Project Team Lead who is responsible for running +midcycles, and skyline-apiserver sessions at the Project Team Gathering for +that cycle (and who is also ultimately responsible for everything else the +project does). + +* You automatically become an ATC by making a commit to one of the + skyline-apiserver deliverables. Other people who haven't made a commit, + but have contributed to the project in other ways (for example, making good + bug reports) may be recognized as "extra-ATCs" and obtain voting privileges. + If you are such a person, contact the current PTL before the "Extra-ATC + freeze" indicated on the current development cycle schedule (which you can + find from the `OpenStack Releases homepage + `_ . + +The current Skyline APIServer project Project Team Lead (PTL) is listed in the +`Skyline APIServer project reference +`_ +maintained by the OpenStack Technical Committee. + +All common PTL duties are enumerated in the `PTL guide +`_. + +New Feature Planning +~~~~~~~~~~~~~~~~~~~~ + +The Skyline APIServer project uses "blueprints" to track new features. Here's a +quick rundown of what they are and how the Skyline APIServer project uses them. + +blueprints + | Exist in Launchpad, where they can be targeted to release milestones. + | You file one at https://blueprints.launchpad.net/skyline-apiserver + + | Examples of changes that can be covered by a blueprint only are: + + * adding a new api + +Feel free to ask in ``#openstack-skyline`` if you have an idea you want to +develop and you're not sure whether it requires a blueprint *and* a spec or +simply a blueprint. + +The Skyline APIServer project observes the following deadlines. For the current +development cycle, the dates of each (and a more detailed description) +may be found on the release schedule, which you can find from: +https://releases.openstack.org/ + +* bp freeze (all bps must be approved by this date) +* new feature status checkpoint + +Task Tracking +~~~~~~~~~~~~~ + +We track our tasks in Launchpad. See the top of the page for the URL of +Skyline APIServer project deliverable. + +If you're looking for some smaller, easier work item to pick up and get started +on, search for the 'low-hanging-fruit' tag in the Bugs section. + +When you start working on a bug, make sure you assign it to yourself. +Otherwise someone else may also start working on it, and we don't want to +duplicate efforts. Also, if you find a bug in the code and want to post a +fix, make sure you file a bug (and assign it to yourself!) just in case someone +else comes across the problem in the meantime. + +Reporting a Bug +~~~~~~~~~~~~~~~ + +You found an issue and want to make sure we are aware of it? You can do so in +the Launchpad space for the affected deliverable: + +* skyline-apiserver: https://bugs.launchpad.net/skyline-apiserver + +Getting Your Patch Merged +~~~~~~~~~~~~~~~~~~~~~~~~~ + +Before your patch can be merged, it must be *reviewed* and *approved*. + +The Skyline APIServer project policy is that a patch must have two +2s before +it can be merged. (Exceptions are documentation changes, which require only a +single +2, for which the PTL may require more than two +2s, depending on the +complexity of the proposal.) Only members of the skyline-core team can vote +2 +(or -2) on a patch, or approve it. + +.. note:: + + Although your contribution will require reviews by members of + skyline-core, these aren't the only people whose reviews matter. + Anyone with a gerrit account can post reviews, so you can ask + other developers you know to review your code ... and you can + review theirs. (A good way to learn your way around the codebase + is to review other people's patches.) + + If you're thinking, "I'm new at this, how can I possibly provide + a helpful review?", take a look at `How to Review Changes the + OpenStack Way + `_. + + There are also some Skyline APIServer project specific reviewing + guidelines in the :ref:`reviewing-skyline-apiserver` section of the + Skyline APIServer Contributor Guide. + +In addition, some changes may require a release note. Any patch that +changes functionality, adds functionality, or addresses a significant +bug should have a release note. You can find more information about +how to write a release note in the :ref:`release-notes` section of the +Skyline APIServer Contributors Guide. + +.. note:: + + Keep in mind that the best way to make sure your patches are reviewed in + a timely manner is to review other people's patches. We're engaged in a + cooperative enterprise here. + +If your patch has a -1 from Zuul, you should fix it right away, because +people are unlikely to review a patch that is failing the CI system. + +* If it's a pep8 issue, the job leaves sufficient information for you to fix + the problems yourself. +* If you are failing unit or functional tests, you should look at the + failures carefully. These tests guard against regressions, so if + your patch causing failures, you need to figure out exactly what is + going on. +* The unit, functional, and pep8 tests can all be run locally before you + submit your patch for review. By doing so, you can help conserve gate + resources. + +How long it may take for your review to get attention will depend on the +current project priorities. For example, the feature freeze is at the +third milestone of each development cycle, so feature patches have the +highest priority just before M-3. These dates are clearly noted on the +release schedule for the current release, which you can find +from https://releases.openstack.org/ + +You can see who's been doing what with Skyline APIServer recently in +Stackalytics: +https://www.stackalytics.io/report/activity?module=skyline-group diff --git a/doc/source/contributor/gerrit.rst b/doc/source/contributor/gerrit.rst new file mode 100644 index 0000000..551bee5 --- /dev/null +++ b/doc/source/contributor/gerrit.rst @@ -0,0 +1,187 @@ +.. _reviewing-skyline-apiserver: + +Code Reviews +============ + +Skyline APIServer follows the same `Review guidelines`_ outlined by the +OpenStack community. This page provides additional information that is +helpful for reviewers of patches to Skyline APIServer. + +Gerrit +------ + +Skyline APIServer 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 APIServer 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 APIServer only needs to support Python 3 runtimes (in particular, +3.8). Our biggest interaction with the stable branches is backporting +bugfixes, where in the ideal case, we're just doing a simple cherry-pick of +a commit from master to the stable branches. You can see that there's some +tension here. + +With that in mind, here are some guidelines for reviewers and developers +that the Skyline APIServer community has agreed on during this phase where we +want to write pure Python 3 but still must support Python 2 code. + +.. _transition-guidelines: + +Python 2 to Python 3 transition guidelines +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +* New features can use Python-3-only language constructs, but bugfixes + likely to be backported should be more conservative and write for + Python 2 compatibilty. + +Unit Tests +---------- + +Skyline APIServer 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-apiserver 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 APIServer 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 check for jobs like pep8, py38, functional + etc that are generic to all the patches and 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 Python constructs and working of + Skyline APIServer. 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. + +* **Type Annotations**: There has been an ongoing effort to implement type + annotations all across Skyline APIServer with the help of mypy tooling. + Certain areas of code already adapt to mypy coding style and it's good + practice that new code merging into Skyline APIServer should also adapt to + it. We, as reviewers, should ensure that new code proposed should include + mypy constructs. + +* **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 tempest 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/index.rst b/doc/source/contributor/index.rst index 00d22a9..90293bc 100644 --- a/doc/source/contributor/index.rst +++ b/doc/source/contributor/index.rst @@ -1,6 +1,29 @@ -=========================== - Contributor Documentation -=========================== +Contributor Guide +================= +In this section you will find information on how to contribute to +skyline-apiserver. Content includes architectural overviews, tips and tricks +for setting up a development environment. + +Getting Started +--------------- .. toctree:: - :maxdepth: 1 + :maxdepth: 2 + + contributing + +Other Resources +--------------- +.. toctree:: + :maxdepth: 3 + + gerrit + releasenotes + +.. only:: html + + Indices and tables + ------------------ + + * :ref:`genindex` + * :ref:`search` diff --git a/doc/source/contributor/releasenotes.rst b/doc/source/contributor/releasenotes.rst new file mode 100644 index 0000000..07c44a7 --- /dev/null +++ b/doc/source/contributor/releasenotes.rst @@ -0,0 +1,105 @@ +.. _release-notes: + +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 APIServer 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. diff --git a/doc/source/index.rst b/doc/source/index.rst index bdb9487..bb40bc3 100644 --- a/doc/source/index.rst +++ b/doc/source/index.rst @@ -62,15 +62,23 @@ Release Notes See https://docs.openstack.org/releasenotes/skyline-apiserver -Information -=========== +Additional reference +-------------------- + +Contents: .. toctree:: :maxdepth: 1 - glossary + common/glossary.rst + .. only:: html + Indices and tables + ------------------ + + Contents: + * :ref:`genindex` - * :ref:`modindex` + * :ref:`search`