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

Add readme file for cases and environ variable for ce-oem-provider (New) #1312

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stanley31huang
Copy link
Collaborator

@stanley31huang stanley31huang commented Jun 24, 2024

Description

Add readme file for cases and environ variable for ce-oem-provider
We would like to have a document to describe which environ variable has been defined in the checkbox-ce-oem provider

AI
The README files are currently generated by a script, and we plan to introduce another PR to facilitate their updates in the future. The idea is to run a script to generate the README files and compare the differences in the GitHub workflow when a PR is created (similar to the tox check). If any differences are found, the script will add a comment to the PR to notify the reviewer.

Resolved issues

N/A

Documentation

N/A

Tests

N/A

Copy link
Collaborator

@pieqq pieqq left a comment

Choose a reason for hiding this comment

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

This must have required a lot of work, unless you had some kind of script to generate the content!

A few comments:

  • In the "Affected Test Cases" sections, I would point directly to the job in the pxu file. You can use relative links, and GitHub will understand them. Doing this will avoid a lot of duplicated data (environ, summary, description, command) that will probably go out of date quickly
  • How do you plan on maintaining this? Are other contributors to ce-oem-provider aware they have to update the associated md files if they introduce a new environment key? Maybe you can add this to the README file.

@stanley31huang
Copy link
Collaborator Author

@pieqq Thanks for the suggestion, I have updated the PR and please see my inline comments

This must have required a lot of work, unless you had some kind of script to generate the content!

A few comments:

  • In the "Affected Test Cases" sections, I would point directly to the job in the pxu file. You can use relative links, and GitHub will understand them. Doing this will avoid a lot of duplicated data (environ, summary, description, command) that will probably go out of date quickly

The reason we generate this kind of markdown files is to have a better view to know how to configure the environ variable.
As you may know that not all of jobs has clear description to describe how to configure it, so we may need to look into the command section.

So the idea is to shows some key information in the markdown file and also have a hyperlink to the source file.

  • How do you plan on maintaining this? Are other contributors to ce-oem-provider aware they have to update the associated md files if they introduce a new environment key? Maybe you can add this to the README file.

I have uploaded the scripts which use to generate the Markdown file and added a section to describe how to update them.
we had a discussion in QA team about how to maintain those markdown file, the idea is add an step in the github workflow to update those markdown file regularly.

@stanley31huang stanley31huang requested a review from pieqq June 27, 2024 08:48
@stanley31huang stanley31huang marked this pull request as draft July 1, 2024 01:31
add readme file for cases and environ
update the readme files
@stanley31huang
Copy link
Collaborator Author

Per discussion with Max few weeks ago, he suggest us not doing changes in the github workflow directly. But running a scripts to check is there any difference for those readme files instead.

So I am going to send another PR to do following actions.

  1. Generate the new readme files
  2. Compare the readme files
  3. Add a comment about the difference of readme files.

So in this PR, it's mainly for adding the readme files. And you could see how is looks like in this branch.

@stanley31huang stanley31huang marked this pull request as ready for review July 15, 2024 07:46
Copy link
Collaborator

@zongminl zongminl left a comment

Choose a reason for hiding this comment

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

As the first step of capturing environ variables into README files, this PR looks good enough.
Some environ variables still missing detailed descriptions, this will be something to be fixed in the future.

@stanley31huang
Copy link
Collaborator Author

@pieqq I have updated the description to have an followup action to implement a script to keep these document up to date. Could you please review this PR again?

@stanley31huang stanley31huang enabled auto-merge (squash) September 2, 2024 08:15
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.

3 participants