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

Update the registry stack PR review process #384

Merged
merged 8 commits into from
May 14, 2024

Conversation

thepetk
Copy link
Contributor

@thepetk thepetk commented Apr 30, 2024

What does this PR do?:

This PR updates the contributing guide, adding a section to describe the process followed by the devfiles team regarding the review of PRs updating stacks content.

Other than the contributing guide, it removes all OWNERS files and moves all related information inside the .github/CODEOWNERS file.

Which issue(s) this PR fixes:

fixes devfile/api#1498
fixes devfile/api#1525

PR acceptance criteria:

  • Contributing guide
    Have you read the devfile registry contributing guide and followed its instructions?
  • Test automation
    Does this repository's tests pass with your changes?
  • Documentation
    Does any documentation need to be updated with your changes?
  • Check Tools Provider
    Have you tested the changes with existing tools, i.e. Odo, Che, Console? (See devfile registry contributing guide on how to test changes)

How to test changes / Special notes to the reviewer:

@thepetk
Copy link
Contributor Author

thepetk commented Apr 30, 2024

WIP Because:

  1. all codeowners need to have write access to the devfile registry repo
  2. the devfile-services-team needs to be the only one with permissions to merge a PR

@thepetk thepetk requested a review from ibuziuk May 8, 2024 15:48
@thepetk thepetk changed the title WIP: Update the registry stack PR review process Update the registry stack PR review process May 8, 2024
@thepetk
Copy link
Contributor Author

thepetk commented May 8, 2024

WIP Because:

  1. all codeowners need to have write access to the devfile registry repo
  2. the devfile-services-team needs to be the only one with permissions to merge a PR

The above blockers were resolved, removed the WIP label

CONTRIBUTING.md Show resolved Hide resolved
@Jdubrick
Copy link
Contributor

Jdubrick commented May 8, 2024

You might need to force push the sign off for the commits to clear DCO @thepetk

@thepetk
Copy link
Contributor Author

thepetk commented May 9, 2024

You might need to force push the sign off for the commits to clear DCO @thepetk

Yeap :) I want to make small update to the CODEOWNERS file so I'll fix it once I push my new commits

OWNERS Outdated Show resolved Hide resolved
Copy link
Member

@michael-valdron michael-valdron left a comment

Choose a reason for hiding this comment

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

@ibuziuk is adding a new ollama stack that will need the OWNERS file refactored as well: #387

Since that PR needs merging by the weeks end, I'd suggest we wait for that change then refactor that OWNERS file into CODEOWNERS here, that way we don't need an extra PR and it still should be completed by sprints end. @thepetk wdyt?

.github/CODEOWNERS Outdated Show resolved Hide resolved
Copy link
Member

@michael-valdron michael-valdron 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, should include someone from team che to approve as well.

@thepetk
Copy link
Contributor Author

thepetk commented May 10, 2024

Looks good to me, should include someone from team che to approve as well.

I've already requested review from @ibuziuk :)

Copy link
Contributor

@Jdubrick Jdubrick left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Looks good to me label May 10, 2024
.github/CODEOWNERS Outdated Show resolved Hide resolved
@openshift-ci openshift-ci bot removed the lgtm Looks good to me label May 14, 2024
@thepetk
Copy link
Contributor Author

thepetk commented May 14, 2024

@michael-valdron as you have requested changes in the past, could you review the updated version of the PR?

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
SIgned-off-by: thepetk <thepetk@gmail.com>
Signed-off-by: thepetk <thepetk@gmail.com>
Copy link
Member

@michael-valdron michael-valdron left a comment

Choose a reason for hiding this comment

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

/lgtm

@openshift-ci openshift-ci bot added the lgtm Looks good to me label May 14, 2024
Copy link
Contributor

@dmytro-ndp dmytro-ndp 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 merge

Copy link

openshift-ci bot commented May 14, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dmytro-ndp, ibuziuk, Jdubrick, michael-valdron, thepetk

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Jdubrick,michael-valdron,thepetk]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@thepetk thepetk merged commit beecb12 into devfile:main May 14, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Looks good to me
Projects
None yet
5 participants