posener / goreadme

Generate readme file from Go doc. Now available as a Github action!
MIT License
213 stars 33 forks source link

Use the correct branch name in the badges #102

Open marksalpeter opened 3 years ago

marksalpeter commented 3 years ago

👋 @posener. We're using this action to generate our readme over here and I noticed that all of our badges were broken (our master branch is called main).

I added a feature that renders the HEAD branch name in the template by default or a branch override flag. I also added a GitHub workflow bage, and a flag to override the default commit message. Let me know what you think.

Finally, the checks appear to be failing, but I don't understand why. All of the tests are passing locally.

posener commented 3 years ago

Hi Mark. Thanks for using goreadme and submitting a fix for the issue. It was not me that was responding, but the goreadme/goaction bots. I should fix this such that it will look like a bot and not like a person responding.

I see the issue you are suggesting on. As for the solution: I think that exec.Command should be used as a last resort. In this case, I think you can use the goaction.Branch() value instead. See https://github.com/posener/goaction/blob/master/goaction.go#L237. Can you update your change and see if it works for you?

marksalpeter commented 3 years ago

Thanks @posener, I'll make the change tomorrow.

marksalpeter commented 3 years ago

:wave: Hello agian @posener!

I referenced this fork by commit SHA in our repo. You can now see all of the features this fork has added in the readme file of the @deliveryhero pipelines repo.

This version uses goaciton.Branch() first and then falls back to git commands for the compiled local version. Let me know what you think when you get the chance.

Thanks again for the feedback 🙏

posener commented 3 years ago

Hi Mark,

I think this PR is currently mixing multiple things. The branch name issue, adding badges, allowing modify commit messages. Usually one would create a different independent PR for each of these.

Scoping back to the branch name issue. I think that the failure you get is because the call uses a the env variables from Github action environment. You should protect the call with if goaction.CI { ... }. You can read more at https://github.com/posener/goaction

marksalpeter commented 3 years ago

Thanks again for the feedback @posener!

You're right, this isn't a very clean PR. I just implemented all of the badge features we needed in one go. It would have been better in hind-sight if I had made separate branches and PRs for the commit message override, the github actions badge, and the branch name fix.

It looks like the repo has changed significantly since my fork and there are merge conflicts now. We'll just keep using my fork for now and if you ever get around to incorporating these features into the action we'll switch back. Feel free to close this PR.

Best, -Mark