ansible / validated-content-discussion

A place for review of validated content candidates and general discussion around validated content.
GNU General Public License v3.0
2 stars 4 forks source link

candidate: infra.convert2rhel #27

Closed r0x0d closed 4 months ago

r0x0d commented 5 months ago

Candidate Name

infra.convert2rhel

Link to Github Repo

https://github.com/redhat-cop/infra.convert2rhel

Review Due By

03/25/2024

CHECKLIST

Business Checks:

Content Viability

Content Duplication

Technical Checks:

Content Conformity

Content Compliance

Content Testing/Validation

alisonlhart commented 5 months ago

@djdanielsson @arnav3000 @ericzolf @sean-m-sullivan Can you take a look at this candidate?

djdanielsson commented 5 months ago

checklist isn't done, but skimming some of the code I think it is good.

r0x0d commented 4 months ago

Hi, it's been some time now... Should I check the boxes myself, or is this job for someone else to do?

djdanielsson commented 4 months ago

it is your job to check the boxes

r0x0d commented 4 months ago

Hi, @heatmiser, could you take a look for the Content Viability section, please? Thanks.

heatmiser commented 4 months ago

@r0x0d All items in Content Viability section are covered.

r0x0d commented 4 months ago

@alisonlhart, all items in check list are done.

alisonlhart commented 4 months ago

@r0x0d A few items I saw during review:

r0x0d commented 4 months ago

Hi, @alisonlhart! Thanks for the comments.

Regarding the galaxy.yml and the CODEOWNERS file, here's the PR that fixes both of them: https://github.com/redhat-cop/infra.convert2rhel/pull/41

Regarding the workflow, you can see in the last line (https://github.com/redhat-cop/infra.convert2rhel/blob/main/.github/workflows/pre-commit.yml#L37) we call pre-commit action to execute pre-commit for us.

Our ansible-lint is being executed via pre-commit hook, that you can take a look at: https://github.com/redhat-cop/infra.convert2rhel/blob/main/.pre-commit-config.yaml#L14

alisonlhart commented 4 months ago

@r0x0d Thank you for pointing me to the correct file! I figured I was just missing it somewhere. After the PR is merged, I believe that meets all checklist criteria. I'll add this to the sign-off list for this week's ACoP call on April 17.

alisonlhart commented 4 months ago

Approved by the ACoP! @r0x0d please reach out to me on slack @allhart to get your access to the infra namespace on Automation Hub set up.

r0x0d commented 4 months ago

Approved by the ACoP! @r0x0d please reach out to me on slack @allhart to get your access to the infra namespace on Automation Hub set up.

Thanks, @alisonlhart!

We just merged https://github.com/redhat-cop/infra.convert2rhel/pull/41 and will be generating a new release soon!