DOC more updates to maintainers' doc #18954

ogrisel · Dec 2, 2020

In particular, mention the possibility to use the GitActions workflow to publish to PyPI.

adrinjalali

Thanks @ogrisel

doc/developers/maintainer.rst

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>

doc/developers/maintainer.rst

cmarmo · Dec 2, 2020

doc/developers/maintainer.rst

+
+  Before building the wheels, make sure that the ``pyproject.toml`` file is
+  up to date and using the oldest version of ``numpy`` for each Python version
+  to avoid ABI incompatibility issues. Moreover, a new line have to be included


Suggested change

to avoid ABI incompatibility issues. Moreover, a new line have to be included

to avoid API incompatibility issues. Moreover, a new line have to be included

This sentence is correct, that is, it should be ABI (Application Binary Interface) incompatibility issues. In particular, we must link the oldest supported numpy version to the binary wheels of scikit-learn to ensure that they are compatible when installing those wheels using a newer numpy version (ABI compatibility).

This is ABI for Application Binary Interface. I will make that more explicit.

cmarmo · Dec 2, 2020

doc/developers/maintainer.rst

+  to avoid ABI incompatibility issues. Moreover, a new line have to be included
+  in the ``pyproject.toml`` file for each new supported version of Python.
+
+4. Once the CD has completed successfully, upload the generated artifacts


CD? Perhaps CI?
Or maybe you could explain what CD means? At least I don't know what it is... :(

https://en.wikipedia.org/wiki/Continuous_delivery

I will rephrase this paragraph and the previous one.

Sure @cmarmo, let me explain carefully.

CD means Continuous Delivery. It is the automated process by which the code is built (wheel), test, and then put into a staging or pre-production environment (Anaconda cloud).

Should we put this information in the maintainers' guide or a link to a proper definition should be enough?

doc/developers/maintainer.rst

Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>

alfaro96

Thank you @ogrisel for your PR!

A first review round, but this is looking good 😄.

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

-First, create a branch, **on your own fork** (to release e.g. `0.999.3`):
+For major releases, do not forget to prepare a Release Highlights page as a
+runnable example and check that its HTML rendering looks correct. These release
+higlights should be linked from the ``whats_new.rst`` file for the new version


Suggested change

higlights should be linked from the ``whats_new.rst`` file for the new version

highlights should be linked from the ``whats_new.rst`` file for the new version

doc/developers/maintainer.rst

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

+       $ git shortlog -s 0.98.33.. | cut -f2- | sort --ignore-case | tr '\n' ';' | sed 's/;/, /g;s/, $//'
+
+     For major releases, make sure that the release highlights example is
+     properly linked from the ``doc/v0.99/whats_new.rst`` file.


Just in case:

Suggested change

properly linked from the ``doc/v0.99/whats_new.rst`` file.

properly linked from the ``doc/whats_new/v0.99.rst`` file.

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

+
+  Before building the wheels, make sure that the ``pyproject.toml`` file is
+  up to date and using the oldest version of ``numpy`` for each Python version
+  to avoid ABI incompatibility issues. Moreover, a new line have to be included


This sentence is correct, that is, it should be ABI (Application Binary Interface) incompatibility issues. In particular, we must link the oldest supported numpy version to the binary wheels of scikit-learn to ensure that they are compatible when installing those wheels using a newer numpy version (ABI compatibility).

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

+  to avoid ABI incompatibility issues. Moreover, a new line have to be included
+  in the ``pyproject.toml`` file for each new supported version of Python.
+
+4. Once the CD has completed successfully, upload the generated artifacts


Sure @cmarmo, let me explain carefully.

CD means Continuous Delivery. It is the automated process by which the code is built (wheel), test, and then put into a staging or pre-production environment (Anaconda cloud).

doc/developers/maintainer.rst

alfaro96

Another review round.

doc/developers/maintainer.rst

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

-       $ git shortlog -s 0.99.33.. | cut -f2- | sort --ignore-case | tr '\n' ';' | sed 's/;/, /g;s/, $//'
+       $ git shortlog -s 0.98.33.. | cut -f2- | sort --ignore-case | tr '\n' ';' | sed 's/;/, /g;s/, $//'
+
+     - For major releases, link the release highlights example from the ``doc/whats_new/v0.99.rst`` file.

   - Update the release date in ``whats_new.rst``


I do not see a release date in the whats_new.rst file 😕.

We forgot on some of them in the past.

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

+.. note::
+
+  Before building the wheels, make sure that the ``pyproject.toml`` file is
+  up to date and using the oldest version of ``numpy`` for each Python version
+  to avoid `ABI <https://en.wikipedia.org/wiki/Application_binary_interface>`_
+  incompatibility issues. Moreover, a new line have to be included in the
+  ``pyproject.toml`` file for each new supported version of Python.


I think that this information is not necessary if we decided to move on with oldest-supported-numpy #18900.

alfaro96 · Dec 2, 2020

doc/developers/maintainer.rst

+  to avoid `ABI <https://en.wikipedia.org/wiki/Application_binary_interface>`_
+  incompatibility issues. Moreover, a new line have to be included in the
+  ``pyproject.toml`` file for each new supported version of Python.
+


I think that we have to add a new step here (before step 4) to mention that the wheels should be uploaded to the staging area using the "Run workflow" form for the Wheel builder workflow.

Nevertheless, if the PR from step 3 is merged to the release branch, this step would not be necessary (i.f.f. the commit message contains [cd build] marker).

However, I do not really like the idea of merging an empty PR to the release brach.

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es> Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>

ogrisel · Dec 2, 2020

@alfaro96 @cmarmo @adrinjalali I pushed more changes to rework the structure of the doc to try to make it easier to follow (especially w.r.t. the branching to prepare the PR).

doc/developers/maintainer.rst

alfaro96

A first review round to the new version @ogrisel 😄.

doc/developers/maintainer.rst

adrinjalali · Dec 2, 2020

doc/developers/maintainer.rst

+
+   .. prompt:: bash $
+
+     git checkout -b 0.99.X


The explanation for this command now misses the key point that the release manager needs to do it either under a different clone from the main repo, or under the same clone, but from and to upstream instead of origin.

I expanded the commands to be more explicit.

doc/developers/maintainer.rst

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

alfaro96

A few nitpicks, but LGTM.

doc/developers/maintainer.rst

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

ogrisel · Dec 3, 2020

Sorry for all the typos... and thank you for the fixes.

adrinjalali

Thanks @ogrisel hopefully the next person doing a release would have fewer changes to this doc ;)

thomasjpfan

It's nice to see the release process get easier :)

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com> Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com> Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

DOC more updates to maintainers' doc

ec58d17

github-actions bot added the Documentation label Dec 2, 2020

ogrisel requested review from alfaro96 and thomasjpfan December 2, 2020 09:47

adrinjalali approved these changes Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

Update doc/developers/maintainer.rst

bc354dc

Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>

ogrisel commented Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

Update doc/developers/maintainer.rst

58016c5

cmarmo reviewed Dec 2, 2020

View reviewed changes

ogrisel commented Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

Apply suggestions from code review

efd3408

Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>

alfaro96 reviewed Dec 2, 2020

View reviewed changes

cmarmo reviewed Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

alfaro96 reviewed Dec 2, 2020

View reviewed changes

ogrisel and others added 3 commits December 2, 2020 14:58

Apply suggestions from code review

773fefa

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es> Co-authored-by: Chiara Marmo <cmarmo@users.noreply.github.com>

Improve structure of the maintainers doc

f141f81

Cosmetics

4e973c7

ogrisel commented Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

ogrisel commented Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

ogrisel commented Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

ogrisel added 2 commits December 2, 2020 16:56

update the checklist and cross-reference it

d0e692b

Refine checklist

ce10a7d

alfaro96 reviewed Dec 2, 2020

View reviewed changes

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

doc/developers/maintainer.rst Outdated Show resolved Hide resolved

Fix bug in ordering

239d4b2

adrinjalali reviewed Dec 2, 2020

View reviewed changes

ogrisel and others added 3 commits December 2, 2020 19:29

Move paragraph + more explicit branching commands

a59b3bf

Apply suggestions from code review

73ac6f3

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

Update doc/developers/maintainer.rst

b3e86ec

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

alfaro96 approved these changes Dec 3, 2020

View reviewed changes

Apply suggestions from code review

eaea674

Co-authored-by: Juan Carlos Alfaro Jiménez <JuanCarlos.Alfaro@uclm.es>

adrinjalali approved these changes Dec 3, 2020

View reviewed changes

thomasjpfan approved these changes Dec 4, 2020

View reviewed changes

thomasjpfan merged commit 4773f3e into scikit-learn:master Dec 4, 2020

ogrisel deleted the update-doc-pypi-upload branch December 9, 2020 09:45

glemaitre mentioned this pull request Dec 22, 2020

Release 0.24.0 #19058

Merged

14 tasks

glemaitre mentioned this pull request Apr 22, 2021

Release 0.24.2 #19954

Merged

12 tasks

	to avoid ABI incompatibility issues. Moreover, a new line have to be included
	to avoid API incompatibility issues. Moreover, a new line have to be included

	higlights should be linked from the ``whats_new.rst`` file for the new version
	highlights should be linked from the ``whats_new.rst`` file for the new version

	properly linked from the ``doc/v0.99/whats_new.rst`` file.
	properly linked from the ``doc/whats_new/v0.99.rst`` file.

Search code, repositories, users, issues, pull requests...

Uh oh!

DOC more updates to maintainers' doc #18954

DOC more updates to maintainers' doc #18954

Uh oh!

Conversation

ogrisel commented Dec 2, 2020

Uh oh!

adrinjalali left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alfaro96 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alfaro96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogrisel commented Dec 2, 2020

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alfaro96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alfaro96 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!