anmol098 / waka-readme-stats

This GitHub action helps to add cool dev metrics to your github profile Readme
MIT License
3.36k stars 550 forks source link

Add CI / CD system to this repository #377

Open aravindvnair99 opened 1 year ago

aravindvnair99 commented 1 year ago

Objectives:

pseusys commented 1 year ago

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 commented 1 year ago

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.

aravindvnair99 commented 1 year ago

@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.

pseusys commented 1 year ago

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 commented 1 year ago

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.

anmol098 commented 1 year ago

@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

pseusys commented 1 year ago

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 commented 1 year ago

@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!

anmol098 commented 1 year ago

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

pseusys commented 1 year ago

@anmol098 great, keep me informed!

anmol098 commented 1 year ago

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

anmol098 commented 1 year ago

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.

pseusys commented 1 year ago

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

pseusys commented 1 year ago

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.

aravindvnair99 commented 1 year ago

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.

pseusys commented 1 year ago

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.