github-action-push-to-another-repository
github-action-push-to-another-repository copied to clipboard
Unit testing
- Added unit testing
- Added separate file for functions
- Setup of the git submodules is made with:
git submodule update --init
- Test can be run within the repo with:
./test/bats/bin/bats test/test.bats
Thanks very much for the MR! I will try to review it soon (weekend or next week). First look is that is good, just need time to read it properly :-)
@cpina You seem very busy, and I don't see a lot of movement in the repository would you be interested on adding me as a mantainer?
This PR felt into other things that I've been doing.
I will have a better look soon. Thanks again for the PR, I should have acted (merge, feedback, etc.) sooner.
When I had a quick look in May I thought that I was in a catch-22 situation: I don't want to change the code without tests, but to implement the tests, in the PR, the code is changing :-) (and I really want to avoid regressions). I've had a look now and it's not as different as I had initially thought.
Regarding maintainer: I'd like to make the repo more sustainable, so I'll think about it (I have some concerns either way). Let's see first where I get with the PR :-) Thanks for offer your help though (and for the good PR with the needed tests and re-factoring).
I will have a better look soon. Thanks again for the PR, I should have acted (merge, feedback, etc.) sooner.
No worries (I always get ghosted on PRs), and forgetting to do things is basically my thing so it's truly alright.
I've had a look now and it's not as different as I had initially thought.
I have purposefully made it as similar as the original as I could just splitting it into functions so it could be tested, further more if you don't like the changes that I propose (that are truly little over at a2e083e01c8506d549d0db9a27115146dfa280aa ) you can just throw away that commit.
Regarding maintainer: I'd like to make the repo more sustainable, so I'll think about it (I have some concerns either way). Let's see first where I get with the PR :-) Thanks for offer your help though (and for the good PR with the needed tests and re-factoring).
It's good to have concerns (trust, communication, etc), but being a lone maintainer is heavy, so even if it's not me (because you don't know me or trust me) you should look into having another maintainer.
I hope I can hear your feedback soon.
@cpina RFC
My thoughts at the moment:
- I really like the approach that you took for the unit tests. If I started the action now I would take this approach!
- I feel uncomfortable with the "catch 22" situation that generates: change the code to add unit tests, but I wanted to add tests to change the code (in #59 I talk about tests in general, not unit tests to avoid changing the code)
- Because of #live and #work I prefer to not commit now to make structural changes in the action. I know that is unlikely that will break things, but it's possible. The action is quite used and I want to avoid regressions (and I know that many people use @main instead of the tagged versions). I don't want to have to fix things if I can avoid breaking them (and, when GitHub changed things causing regressions, I fixed them as quickly as I could).
- I don't think that I should add a maintainer at this point
Side topic: My plans for when I get into this: I want to add an explanation (in the FAQ) in the docs on how to use .gitignore (as mentioned in some issue or MR) to avoid copying / overwriting files. I thought that it was a clever way, and .gitignore has many, many features on includes and excludes. Adding options to the action for this would be endless (exclude a directory, include subdirectories, exclude files..), if it can use .gitignore we can cover many things (which I can if I remember correctly).
I also want to add a list of related github-actions (other actions that serve the same or similar purpose). Not all of them can do everything, and it would be useful to have a bit of a list for when one of them does not fit a use case.
@pwall2222 : an idea is that you fork this one, merge your MR and I add yours (and others!) in the docs. If you do: ping me here and I'll add this section in the docs. I guess that this is probably not your desired way (else you would have already done) but I think that's a way to move forward.
Perhaps, when I have time to write integration tests to ensure that nothing gets broken then I even merge it back? Who knows!
For now I'm happy to leave the MR open because I still find it interesting!
Feel free to get in touch for any comments or thoughts!