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

added docs for git, versions, and changelog #25

Closed
wants to merge 3 commits into from
Closed

Conversation

mmccarty
Copy link
Contributor

@mmccarty mmccarty commented Jun 15, 2021

This describes a slightly more conservative approach than what we discussed, but this is taken from the RAPIDS team. I thought perhaps folks on the @nv-legate/core-team would find it more comfortable.

@mmccarty
Copy link
Contributor Author

Weird. The docs built locally. Thanks @manopapad

@manopapad
Copy link
Contributor

I am generally fine with this workflow, although I would prefer the development branch to have a stable identifier (something like develop). Does anyone have comments on using tags instead / in addition to named branches for releases?

@marcinz
Copy link
Collaborator

marcinz commented Jun 25, 2021

I wonder how a constant develop tag interacts with PRs. Say that I create a PR vs a current release branch, then this PR will remain against this branch even if a new one is created. If I create a PR against a develop branch, it can accidentally become a sliding target. I think being explicit in targeting PRs to a particular release could be beneficial.

I think using tags for releases has a similar issue. From what I understand, we want to potentially work on a frozen release branch, which means it should remain a branch as it is still "alive."

@mmccarty
Copy link
Contributor Author

In general, we should avoid PRs against long lived development branches. As long as we are all PRing to the release branch, we avoid the sliding target.

I think being explicit in targeting PRs to a particular release could be beneficial.

Which is the proposal here.

@mmccarty
Copy link
Contributor Author

As of today, the new-core branch is 44 commits ahead, 5 commits behind nv-legate:master.

If everyone is good with this workflow, I propose we make new-core the release branch in the main repo.

Thoughts?

@lightsighter
Copy link
Contributor

If everyone is good with this workflow, I propose we make new-core the release branch in the main repo. Thoughts?

I think I'm good with this once the new-core is back to functional parity with the existing core. Performance regressions are probably ok and can be worked through, but we shouldn't break anything that we have working right now. @magnatelee to comment as well.

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 1, 2021

Just to clarify: I'm proposing we create a release branch here of new-core that is not frozen, which would mean it is still under active development and can be broken. The dev team could then test, make smaller PRs to that release branch, collaborate, etc. User's would continue to install legate from main until we roll out a frozen release that we have verified.

@lightsighter
Copy link
Contributor

Ok, no arguments from me then.

@magnatelee
Copy link
Contributor

I'm fine with making new-core a release branch. We'll need to do the same for Legate NumPy, as I keep a version consistent with the new core under the same new-core branch in my fork. What's the name of the release branch going to be?

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 2, 2021

That would depend on the target release date. We haven't settled on a cadence so we can pick whatever we want. Since we are using CalVer, we just need to pick a month. For example, if we pick August the branch would be branch-21.08.

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 2, 2021

@marcinz Do you know why the build is failing? The logs just show...

Already up to date.
Error: Process completed with exit code 2.

@marcinz
Copy link
Collaborator

marcinz commented Jul 2, 2021

@marcinz Do you know why the build is failing? The logs just show...

There were issues with the build script at the time when you branched off. The CI uses the build scripts from the point when you created the branch. There was an issue with how the commit for the pull request was being checked out, and I think this is the issue here. You have two options. You could rebase this branch, and then you'll get the latest version of CI, or you could just merge it and let CI run on the merge.

@marcinz
Copy link
Collaborator

marcinz commented Jul 2, 2021

Talking about CI, this will complicate CI a bit. Currently, CI in Pandas and Numpy builds against master in legate.core. What would be a reliable way to decide against which legate.core to build in the new setup?

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 2, 2021

No problem. I'll rebase.

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 8, 2021

Anything further here?

@mmccarty
Copy link
Contributor Author

mmccarty commented Jul 9, 2021

Talking about CI, this will complicate CI a bit. Currently, CI in Pandas and Numpy builds against master in legate.core. What would be a reliable way to decide against which legate.core to build in the new setup?

@marcinz - Sorry, I missed this. We would need to keep them all on the same release branch. Or we could keep them in the same repo.

@magnatelee
Copy link
Contributor

@mmccarty I'm ready to rename and push the new-core branches. Let's aim at doing a release on August. (so we will go with branch-21.08 as you proposed.)

@marcinz I'm about to move the new-core branches to the main repositories, all under the same name (branch-21.08). Will the CI work correctly for them? Also, is it possible to skip tests when the repository doesn't have the right branch? Legate Pandas is yet to be ported to the new API, so the repository won't have the release branch for a while. (I guess I could push one without any changes, but that would just break right away.)

@manopapad
Copy link
Contributor

I added a note about (mostly) using PRs for making changes.

@mmccarty
Copy link
Contributor Author

Closing this PR and opening a new on to branch-21.10. We should follow any open discussions here in an issue.

@mmccarty mmccarty closed this Aug 20, 2021
jjwilke pushed a commit to jjwilke/legate.core that referenced this pull request Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants