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

chore: deprecate "releases" branch by providing clear messaging #12375

Merged
merged 7 commits into from
Aug 13, 2024

Conversation

BigLep
Copy link
Member

@BigLep BigLep commented Aug 10, 2024

This is being done as part of of #12374 . There are additional steps that need to be taken though per that tracking issue.

@BigLep BigLep requested a review from rjan90 August 10, 2024 00:15
@BigLep BigLep self-assigned this Aug 10, 2024
@BigLep BigLep added the skip/changelog This change does not require CHANGELOG.md update label Aug 10, 2024
@BigLep
Copy link
Member Author

BigLep commented Aug 10, 2024

@rjan90 : feel free to take this one over or go a different path. The link to https://github.com/filecoin-project/lotus/blob/master/LOTUS_RELEASE_FLOW.md would need to change if we move that file.

@rjan90
Copy link
Contributor

rjan90 commented Aug 12, 2024

It looks like the contents of the makefile was deleted unintentionally here: cbb76cf#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52, which is why the checks are failing. I will push a change that restores the contents.

restore contents of makefile, which seems to have been deleted accidentally
@BigLep
Copy link
Member Author

BigLep commented Aug 12, 2024

It looks like the contents of the makefile was deleted unintentionally here: cbb76cf#diff-76ed074a9305c04054cdebb9e9aad2d818052b07091de1f20cad0bbac34ffb52, which is why the checks are failing. I will push a change that restores the contents.

I actually intentionally deleted the makefile contents so that there was a very clear error message if anyone does make *. Let me know if you disagree with the approach. If we go with it, I guess I'll need to also remove some of the checks in this branch. I'll write out a list of the other things would need to do.

@rjan90
Copy link
Contributor

rjan90 commented Aug 12, 2024

2024-08-12: Discussed verbally in Zoom that deleting the makefile contents here makes sense. Next steps:

  1. Do some additional manual testing, and potentially make this error out/ make it even more verbose to ensure that people see the message
  2. Write suggestions on how people should now get the latest tags for node and miner
  3. Ask someone that is running scripts building their binary from the releases branch, and get a 👍 that they see the error, and that the new way of getting the latest tags works for them.

@BigLep
Copy link
Member Author

BigLep commented Aug 12, 2024

potentially make this error out/ make it even more verbose to ensure that people see the message

I have taken a stab at this with fe42667

Write suggestions on how people should now get the latest tags for node and miner

I did this in #12322 . See https://github.com/filecoin-project/lotus/pull/12322/files#diff-fb3c9ffe7e0886ba95ce77f3809358fd6aa0ca3994f1bd45195d4ce178d68ca4R157

@BigLep BigLep marked this pull request as ready for review August 12, 2024 16:26
Makefile Show resolved Hide resolved
Co-authored-by: Phi-rjan <[email protected]>
Makefile Outdated Show resolved Hide resolved
Copy link
Contributor

@rjan90 rjan90 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Just one additional suggestion on making it exit with a non-zero status code.

Co-authored-by: Phi-rjan <[email protected]>
@BigLep BigLep merged commit 147e5de into releases Aug 13, 2024
2 checks passed
@BigLep BigLep deleted the biglep/deprecate-releases-branch branch August 13, 2024 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip/changelog This change does not require CHANGELOG.md update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants