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.

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

Jump to bottom

CI: Use 'main' branch instead of 'master'. #191

Merged
merged 1 commit into from
Dec 2, 2021
Merged

CI: Use 'main' branch instead of 'master'. #191

merged 1 commit into from
Dec 2, 2021

Conversation

edmooring
Copy link
Contributor

See #183 for the underlying issue.

Signed-off-by: Ed Mooring [email protected]

@edmooring
CI: Use 'main' branch instead of 'master'.
138c844 
Signed-off-by: Ed Mooring <[email protected]>

Point to the right action.

CI: run compliance on pushes as well.

Rvert.

Update to point to main rather than master.
arnopo
arnopo approved these changes Nov 26, 2021
View reviewed changes
Copy link
Contributor

@arnopo arnopo left a comment

Choose a reason for hiding this comment

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

I Tested on my own github:

  • create the main branch
  • integrate you PR and switch the default branch to main branch
  • create a pull request to verify the CI

Everything is working fine
I created a "main" branch. Could you rebase your PR on it before you merge it? this should avoid to break the CI until you switch the default branch to the main branch

Thanks

@edmooring edmooring linked an issue Nov 27, 2021 that may be closed by this pull request
replace master/slave terminology #183
Closed
@edmooring
Copy link
Contributor Author

I'm not sure why you created the "main" branch. GitHub provides a lot of support for renaming the default branch, including redirection of pull requests. I have tested the process on my local open-amp repo.

  1. I created a pull request and verified that CI and compliance ran against it.
  2. Renamed "master" to "main".
  3. Force pushed to my pull request branch. CI and compliance were not run.
  4. Updated the configuration files in .github/workflows and pushed to main.
  5. Force pushed to my pull request branch again. CI and compliance run.

So it looks like the only problem with CI is the window between steps 2 and 4. If we do those two steps close together, I don't think we'll run into any trouble. This would let us take advantage of the GitHub support for this, including providing people with instructions on how to bring their local repositories in sync with us after the rename.

I think we should delete the current "main" branch and rename the "master branch" just before merging the configuration file changes.

@arnopo
Copy link
Contributor

arnopo commented Dec 2, 2021

I'm not sure why you created the "main" branch. GitHub provides a lot of support for renaming the default branch, including redirection of pull requests. I have tested the process on my local open-amp repo.

  1. I created a pull request and verified that CI and compliance ran against it.
  2. Renamed "master" to "main".
  3. Force pushed to my pull request branch. CI and compliance were not run.
  4. Updated the configuration files in .github/workflows and pushed to main.
  5. Force pushed to my pull request branch again. CI and compliance run.

So it looks like the only problem with CI is the window between steps 2 and 4. If we do those two steps close together, I don't think we'll run into any trouble. This would let us take advantage of the GitHub support for this, including providing people with instructions on how to bring their local repositories in sync with us after the rename.

I think we should delete the current "main" branch and rename the "master branch" just before merging the configuration file changes.

No worries, If you prefer this way, that ok for me. do you make it or do you want that i merge the PR and change the branch name?

@edmooring
Copy link
Contributor Author

@arnopo, I will take care of it.

@edmooring edmooring merged commit 54b797d into OpenAMP:master Dec 2, 2021
@arnopo arnopo added this to the Release V2022.04 milestone May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@arnopo arnopo arnopo approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
Release V2022.04
Development

Successfully merging this pull request may close these issues.

replace master/slave terminology
2 participants
@edmooring @arnopo