nelsonic commented 2 years ago

while writing usage example docs, I noticed that Gogs.push/1 does not require the org name as an argument. While this currently works, because of the "flat" cloning ... and would probably work fine for the project we're using this package in - because we are actually going to abstract the concept of "orgs" away in practice ... the issue is we don't want a published module/package to have an inconsistent API. 🤦‍♂️ (my bad!)

Todo ✅

[x] review all the functions in https://github.com/dwyl/gogs/blob/main/lib/gogs.ex and https://github.com/dwyl/gogs/blob/main/lib/helpers.ex 🔍
- [x] If the org_name is implicit, we need to update the function to require it to be passed in explicitly! 📝
[x] Update all the tests, typespecs, @doc comments, usage example [README.md] and actual implementation code to reflect that we want to always require both org_name and repo_name so that we use the org_name for name-spacing on the Elixir/Phoenix server. ⬆️
[x] Publish a new version to Hex.pm e.g: v1.0.0 🚀

@SimonLab, I know you're working in Elm Land right now ... 🌳 so there is a context-switch involved in me dragging you into looking at this. 🙏 But this really needs a fresh pair of eyes on it. Mine eyes have stared at it too long. 🙄 I think once this is done and we've published a new version, we can ship it in the "consuming" app. 🍕

nelsonic commented 2 years ago

Looks like commit/2: https://hexdocs.pm/gogs/Gogs.html#commit/2 is another problem child. 🙄 @SimonLab thanks for taking a look at this. Really appreciate your feedback. 🙏

I'm going to write a few more tests. Are you creating a branch for this work or am I? 💭

SimonLab commented 2 years ago

@nelsonic I can create a new branch, I need to go over the repo again to make sure I don't misunderstand the issue. So let me know if you ahead of me and have created the branch

nelsonic commented 2 years ago

I've got a branch. Pushing up now. Then we can discuss on our 09:30 standup after you've had another 6 mins to get your coffee. ☕ 😜

nelsonic commented 2 years ago

Busy working on this in https://github.com/dwyl/gogs/pull/25/files 👨‍💻 ⏳ Going to need an espresso for this one ... ☕

nelsonic commented 2 years ago

OK. got espresso + green tea (and caught up with my notification/emails) so now I have a block of focus time for this! 🍅

Going to:

make sure that the local_branch_create/1 function tests are passing before I "break" anything else ...

add more tests so that mock: false gets 100% coverage. i.e. real world end-to-end tests. Currently:

Randomized with seed 358681
----------------
COV    FILE                                        LINES RELEVANT   MISSED
100.0% lib/git_mock.ex                                54        7        0
100.0% lib/gogs.ex                                   177       36        0
100.0% lib/helpers.ex                                105       13        0
100.0% lib/http.ex                                   101       16        0
31.3% lib/httpoison_mock.ex                         103       16       11
[TOTAL]  87.5%
----------------

So pretty close.

start refactoring the GogsHelpers.local_git_repo/1 function to require org_name that will break a bunch of tests because it's one of the core helpers. But once everything is fixed we should be on-track. 🤞

@SimonLab LMK if you want to remote-pair on any of this. I will be on our Zoom call. Just join if you're free. 👍

nelsonic commented 2 years ago

New version published to Hex.pm now requires org_name. e.g: https://hexdocs.pm/gogs/0.9.0/GogsHelpers.html#local_repo_path/2

nelsonic commented 2 years ago

@SimonLab PR #25 assigned to you for final review/merge. 🙏

dwyl / gogs

Inconsistency: Some Functions don't Require `org_name` ... 🤦‍♂️ #24

Todo ✅