waka-readme-stats icon indicating copy to clipboard operation
waka-readme-stats copied to clipboard

Add CI / CD system to this repository

Open aravindvnair99 opened this issue 2 years ago • 16 comments
trafficstars

Objectives:

  • Spend less time reviewing PRs
  • Automatically show preview README with all options enabled based on changes for each PR
  • Having such a system can also help us queue PRs as necessary to merge one after the other

aravindvnair99 avatar Feb 19 '23 11:02 aravindvnair99

Just to mention: manual reviewing changes shouldn't be a provlem now. All you have to do is:

  1. Copy .env.example to some other file, preferrably having .env extension so it will be ignored by git, let's say conf.env for example.
  2. Change INPUT_WAKATIME_API_KEY and INPUT_GH_TOKEN in your conf.env file to your WakaTime API token and GitHub API token respectively.
  3. Run make run-locally ENV=conf.env in the project root.
  4. Enjoy the changes directly in your GitHub profile.

pseusys avatar Feb 19 '23 13:02 pseusys

As this action requiers user GitHub API token, I think the only possible solution for testing would be be:
Put someone's GitHub API token as a repository secret and automatically run action for this user on every PR.
But this will be a great security hole: GitHub token for this action requires write permission, so anyone will be able to do literally anything with it.
I could've think of some dry run solution, but I'd suggest doing it after #371 - because it makes code navigation and understanding really easier. And also adds a codestyle to follow.

pseusys avatar Feb 19 '23 13:02 pseusys

@pseusys We can create a dummy GitHub user and use that profile's secret. We don't have to use an actual individual's profile. If #371 is ready and tested, we can merge that and then come to the CI / CD system.

aravindvnair99 avatar Feb 19 '23 19:02 aravindvnair99

We'd better add a kind of dry-run option that will be able to run without user password.
If you have some spare time, that would be great if you tested #371 locally once more.
I've tested it on my own, but I'd like to be sure.

pseusys avatar Feb 19 '23 20:02 pseusys

It would be also great if you could take a look at #375 as it appears to solve many issues and so it would be gread to merge it ASAP as well.

pseusys avatar Feb 19 '23 22:02 pseusys

@aravindvnair99 @pseusys This is the good feature which can be introduced to test the changes on PR. Approach i can think of

We can introduce a new env variable flag for testing which will run the action with all options enabled and when our module is about to write readme on actual github repo with the env flag enabled the module will just print the output on console. So this way we don't need github token with write permission only read permission can suffice ... Other approach i can think of is we can mock all the external api calls and assert the output which we'll get at the end

Feel free to ask question Thanks

anmol098 avatar Feb 20 '23 04:02 anmol098

Yeah, I like the first solution. In addition we can make the output more verbose for this dry run. I'll try to implement it, but I still suggest it after #371.

pseusys avatar Feb 20 '23 09:02 pseusys

@anmol098 so, we'll need a sample GitHub read token together with WakaTime read token in repository secrets.
I think, maybe we should use yours? Your profile seems to be very rich with data for the action testing!

pseusys avatar Feb 22 '23 17:02 pseusys

Ok perfect I'll add these 2 tokens to repository secret 👍

anmol098 avatar Feb 22 '23 18:02 anmol098

@anmol098 great, keep me informed!

pseusys avatar Feb 22 '23 20:02 pseusys

@pseusys you can check my latest PR #384 feel free to make changes

anmol098 avatar Feb 24 '23 21:02 anmol098

with last issue #398 for file not found exception. we need to make changes to the workflow how we test the action. currently in ci.yml we just test the python code and send the output as a comment.

new approach could be build and publish the docker image with the tag as PR number then pull the docker image in that docker image run action with all flags enabled and pass the output as a comment.

anmol098 avatar Feb 28 '23 10:02 anmol098

Or better do both and compare the results because we shouldn't forget about local running possibility.

pseusys avatar Feb 28 '23 12:02 pseusys

I would also suggest publishing the Docker image for every pull request (named after the pull request, of course) for us and users to be able to test the action not on main branch only. After the pull request is merge, the image can be deleted.

pseusys avatar Mar 01 '23 20:03 pseusys

Considering we got #423 as well. We need to prioritise this. I agree with doing both, but primary should be GitHub action as that's what most people are using. I haven't come across a local user yet although we might have a few.

aravindvnair99 avatar Mar 13 '23 03:03 aravindvnair99

I would suggest the following solution:

  1. Refactor build_image and ci workflows, combine them into build_and_test (or something).
  2. In this workflow create image building and publishing job.
  3. Create jobs for local and Docker (pulled from registry) debug running - then comparing outputs and publishing results in comments if they match.
  4. Optionally write some test jobs - and run them after debug running jobs.
  5. Optionally create another workflow, being triggered on branch deletion - for unpublishing Docker image associated with this branch from registry. This will never affect master as master never gets deleted.

Unfortunately, I won't be able to deal with it soon.

pseusys avatar Mar 13 '23 04:03 pseusys