Skip to content

Refactoring gitwash #23588

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.

Sign up for GitHub

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

Merged
tacaswell merged 5 commits into from
Dec 5, 2022
Merged

Refactoring gitwash #23588

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0600dae
Refactoring gitwash
melissawm Aug 2, 2022
7c1e037
Addressing review comments
melissawm Aug 28, 2022
e592e7a
Addressing more review suggestions
melissawm Aug 29, 2022
270fdde
Reorganizing content
melissawm Dec 1, 2022
1b7510d
Addressing more review comments
melissawm Dec 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Addressing review comments
  • Loading branch information
@melissawm
melissawm committed Dec 1, 2022
commit 7c1e0378de362d6f24466941f98507134728e6cb
19 changes: 11 additions & 8 deletions doc/devel/coding_guide.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@
Pull request guidelines
***********************

Pull requests (PRs) are the mechanism for contributing to Matplotlib's code and
documentation.
`Pull requests (PRs) on GitHub <https://docs.github.com/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/about-pull-requests>`__
Comment thread
melissawm marked this conversation as resolved.
Outdated
are the mechanism for contributing to Matplotlib's code and documentation.

It is recommended to check that your contribution complies with the following
rules before submitting a pull request:

* If your pull request addresses an issue, please use the title to describe the
issue and mention the issue number in the pull request description to ensure
that a link is created to the original issue.

* All public methods should have informative docstrings with sample usage when
appropriate. Use the `numpy docstring standard
<https://numpydoc.readthedocs.io/en/latest/format.html>`_.
issue (e.g. "Add ability to plot timedeltas") and mention the issue number
in the pull request description to ensure that a link is created to the
original issue (e.g. "Closes #8869" or "Fixes #8869"). This will ensure the
original issue mentioned is automatically closed when your PR is merged. See
`the GitHub documentation <https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue>`__
for more details.

* Formatting should follow the recommendations of PEP8_, as enforced by
flake8_. You can check flake8 compliance from the command line with ::
Expand All @@ -45,6 +45,9 @@ rules before submitting a pull request:
.. __: https://github.com/psf/black/issues/148
.. __: https://github.com/psf/black/issues/1984

* All public methods should have informative docstrings with sample usage when
appropriate. Use the :ref:`docstring standards <writing-docstrings>`.

* Each high-level plotting function should have a simple example in the
``Example`` section of the docstring. This should be as simple as possible

@timhoffm timhoffm Aug 26, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we drop this. We don't consistently have this in our current docs.

Examples calls are usually not very instuctive without seeing the visual output. And we have the sphinx-gallery mini galleries at the bottom of the docstring.

We don't consistently have this in our current docs, and I

If we want this, we should move it to https://matplotlib.org/stable/devel/documenting_mpl.html#writing-docstrings

@melissawm melissawm Oct 11, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you're right - and this is actually pretty well explained already in that page. Let me know if what I wrote makes sense, or if we should remove entirely.

to demonstrate the method. More complex examples should go in the
Expand Down
12 changes: 5 additions & 7 deletions doc/devel/development_workflow.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ In what follows we'll refer to the upstream Matplotlib ``main`` branch, as
branch".
* Name your branch for the purpose of the changes - e.g.
``bugfix-for-issue-14`` or ``refactor-database-code``.
* If you get stuck, reach out on Gitter or the Mailing list.
* Ask for code review!
* If you get stuck, reach out on Gitter or
`discourse <https://discourse.matplotlib.org>`__.
* Ask for a code review!
Comment thread
melissawm marked this conversation as resolved.
Outdated

This way of working helps to keep work well organized, with readable history.
This in turn makes it easier for project maintainers (that might be you) to see
Expand Down Expand Up @@ -155,8 +156,8 @@ Say if there is anything you'd like particular attention for - like a
complicated change or some code you are not happy with.

If you don't think your request is ready to be merged, just say so in your pull
request message. This is still a good way of getting some preliminary code
review.
request message and use the "Draft PR" feature of GitHub. This is a good way of
getting some preliminary code review.

Some other things you might want to do
======================================
Expand All @@ -173,9 +174,6 @@ To see a linear list of commits for this branch::

git log

You can also look at the `network graph visualizer <https://github.com/matplotlib/matplotlib/network>`_ for your github
repo.

.. _rebase-on-trunk:

Rebasing on trunk
Comment thread
melissawm marked this conversation as resolved.
Outdated
Expand Down