Contributor
NOTE: I'm not sure if passing a Name GitReference is right, or if I should pass a full GitReference.
For example, deleting a branch works with this code: deleteReference auth owner repo "heads/some-branch". If I were required to pass a full GitReference, I'd have to either fake one or somehow have fetched it via a prior API call. This limitation is why I did what I did here, FWIW.
Contributor
There is no Name GitReference in the lib. Either use Text, because gitReferenceRef :: Text, or change the type of latter.
Contributor
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As commented previously
Contributor Author
Gotcha, I'll just take Text, I think.
I might've misunderstood the philosophy behind Name. I treated it as a tagged Text to avoid errors (e.g. transposition) and didn't think it mattered what you were naming (in this case a GitReference).
Is there any convention for what can or cannot be named? This code does function FWIW, even though there "is no Name GitReference in the lib", which I'm not sure I totally understand.
Contributor
Contributor Author
OK, if I'm understanding correctly: you don't want to just toss around Name X as a tagged Text, you prefer to introduce them by having a field on X that represents its name, then that can be a Name X, and then you can use it elsewhere. 👍
Do you have any concerns about package API compatibility? I think changing the type of gitReferenceRef would require a major bump vs either this PR as-is or passing a Text, which would be minor. I'll go ahead and do it, just wanted to raise the point.
Also, total sidenote: there can exist a GitCommit without sha!? I just noticed in your link that field is Maybe. That's crazy.
Contributor
breaking changes is fine
Contributor Author
This is ready for re-review.
Contributor Author
👋 Hi there, I just wanted to check that you're not waiting on me for anything here?
Contributor
I missed that for github-0.24. Will merge soon-ish.
Contributor Author
Did this get missed for github-0.25 too?
Contributor
amended and merged as part of #446, thanks! Sorry for long wait.