Skip to content
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.

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

common: update RELEASE_STEPS.md file #5822

Merged
merged 1 commit into from
Jul 26, 2023
Merged

Conversation

osalyk
Copy link
Contributor

@osalyk osalyk commented Jul 25, 2023

This change is Reviewable

@osalyk osalyk added the sprint goal This pull request is part of the ongoing sprint label Jul 25, 2023
@codecov
Copy link

codecov bot commented Jul 25, 2023

Codecov Report

Merging #5822 (d903053) into master (514c7d8) will decrease coverage by 0.01%.
Report is 5 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #5822      +/-   ##
==========================================
- Coverage   71.01%   71.00%   -0.01%     
==========================================
  Files         131      131              
  Lines       19175    19175              
  Branches     3193     3193              
==========================================
- Hits        13617    13616       -1     
- Misses       5558     5559       +1     

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @osalyk)


RELEASE_STEPS.md line 7 at r1 (raw file):

After following steps 1-3 you should have 2 local commits. The first one is a new, tagged version
of the PMDK repository. The second commit is required to restore a "default" state of the repository
(for more details, on why this is required, please see section ["For curious readers"](#7-for-curious-readers)).

Suggestion:

#8-for-curious-readers

RELEASE_STEPS.md line 75 at r1 (raw file):

    git push upstream stable-$VER
  • create PR from stable-$VER to the next stable branch (or to master, if the release is from the last stable branch)

I understand that the proposed order is stable-$VER -> master.
So, what is the purpose of the first git push upstream $VERSION command?


RELEASE_STEPS.md line 96 at r1 (raw file):

- [Google group](https://groups.google.com/g/pmem )

## 6. Later, for major/minor release

Suggestion:

## 7. Later, for major/minor release

RELEASE_STEPS.md line 101 at r1 (raw file):

  add a new tag ("$VER") in file `data/releases_linux.yml` based on previous tags in this file.

## 7. For curious readers

Suggestion:

## 8. For curious readers

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @janekmi)


RELEASE_STEPS.md line 7 at r1 (raw file):

After following steps 1-3 you should have 2 local commits. The first one is a new, tagged version
of the PMDK repository. The second commit is required to restore a "default" state of the repository
(for more details, on why this is required, please see section ["For curious readers"](#7-for-curious-readers)).

Done.


RELEASE_STEPS.md line 75 at r1 (raw file):

Previously, janekmi (Jan Michalski) wrote…

I understand that the proposed order is stable-$VER -> master.
So, what is the purpose of the first git push upstream $VERSION command?

This command pushes out the tag


RELEASE_STEPS.md line 96 at r1 (raw file):

- [Google group](https://groups.google.com/g/pmem )

## 6. Later, for major/minor release

Done.


RELEASE_STEPS.md line 101 at r1 (raw file):

  add a new tag ("$VER") in file `data/releases_linux.yml` based on previous tags in this file.

## 7. For curious readers

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)


RELEASE_STEPS.md line 76 at r2 (raw file):

    git push upstream stable-$VER
  • create PR from stable-$VER to the next stable branch (or to master, if the release is from the last stable branch)

Suggestion:

  ```bash
    # push the tag
    git push upstream $VERSION
    # create and push a stable branch
    git checkout -b stable-$VER
    git push upstream stable-$VER
  • create PR from stable-$VER to the next stable branch (or to master, if the release is from the last stable branch)


<!-- Sent from Reviewable.io -->

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @janekmi)


RELEASE_STEPS.md line 76 at r2 (raw file):

    git push upstream stable-$VER
  • create PR from stable-$VER to the next stable branch (or to master, if the release is from the last stable branch)

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @osalyk)


RELEASE_STEPS.md line 71 at r3 (raw file):

  ```bash
    # push the tag
    git push upstream $VERSION

Can we push a tag before creating the stable branch?
It seems that the commit referenced by the tag would not exist on the upstream at this point.


RELEASE_STEPS.md line 81 at r3 (raw file):

  ```bash
    git push upstream HEAD:stable-$VER $VERSION

This command pushes the tag and the release commits in one go, right?

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed (commit messages unreviewed), 2 unresolved discussions (waiting on @osalyk)

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @osalyk)

@osalyk osalyk force-pushed the release-steps branch 3 times, most recently from e50c241 to 48bca30 Compare July 26, 2023 10:31
Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 2 unresolved discussions (waiting on @janekmi)


RELEASE_STEPS.md line 71 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Can we push a tag before creating the stable branch?
It seems that the commit referenced by the tag would not exist on the upstream at this point.

Yes.


RELEASE_STEPS.md line 81 at r3 (raw file):

Previously, janekmi (Jan Michalski) wrote…

This command pushes the tag and the release commits in one go, right?

Right

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)


RELEASE_STEPS.md line 81 at r5 (raw file):

  - create PR to the new stable-$VER branch
- for patch release:
  - create PR to proper stable branch

Not the most important but please not the newline introduced in the suggestion. I find this document very "crowded".

Suggestion:

  ```bash
    # push the tag
    git push upstream $VERSION
  • for a major/minor release:
    • create a stable-$VER branch on the upstream repository
    • create a pull request to the new stable-$VER branch
  • for a patch release:
    • create PR to a proper stable branch


<!-- Sent from Reviewable.io -->

Copy link
Contributor Author

@osalyk osalyk left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 1 files reviewed, 1 unresolved discussion (waiting on @janekmi)


RELEASE_STEPS.md line 81 at r5 (raw file):

Previously, janekmi (Jan Michalski) wrote…

Not the most important but please not the newline introduced in the suggestion. I find this document very "crowded".

Done.

Copy link
Contributor

@janekmi janekmi left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r6, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @osalyk)

@janekmi janekmi added this to the 2.0.0 milestone Jul 26, 2023
@janekmi janekmi merged commit 8a23e22 into pmem:master Jul 26, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
sprint goal This pull request is part of the ongoing sprint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants