Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add listTagPackages #2

Merged
merged 2 commits into from
Jul 19, 2020
Merged

add listTagPackages #2

merged 2 commits into from
Jul 19, 2020

Conversation

TristanCacqueray
Copy link
Contributor

No description provided.

@juhp
Copy link
Owner

juhp commented Jul 17, 2020

Thanks for this.
How do you feel about just returning NVR's? But maybe it is cleaner/safer to keep name, ver, rel separate like you are doing?
I suppose it depends on common usage too.

Basically it looks okay to me: I am slightly uncomfortable with calling it Package, maybe KojiPackage would be better?
Maybe also I should add I am not very happy with my current API, I was lazy to use datatypes heavily,
but moving in that direction would certainly makes such naming choices clearer
(the whole Koji web API seems rather over-complicated and not pretty either, so fleshing out a nicer API is tricky).

@TristanCacqueray
Copy link
Contributor Author

Thank you for the feedback.
NVR sounds good enough since pkgtreediff is already decoding it properly. New version renames the type to KojiPackage.

This change enables querying the latest packages of a koji tag.
@juhp
Copy link
Owner

juhp commented Jul 18, 2020

Thanks

Also just for reference this is how I am using koji-hs currently in fbrnch:
https://github.com/juhp/fbrnch/blob/master/src/Koji.hs
I am still thinking of current koji-hs as a very low-level API, as we gradually use it more we can push common idioms to Fedora.Koji (even I found having BuildID and TaskID exposed outside a bit tedious at times).

I will merge this.
Just one more thing: can we call it kojiListTaggedBuilds?
Then probably it makes more sense to rename KojiPackage to KojiBuild (and similarly readPackage)?

@TristanCacqueray
Copy link
Contributor Author

Alright, the new names look much better!

@juhp juhp merged commit 154e0bb into juhp:master Jul 19, 2020
@juhp
Copy link
Owner

juhp commented Jul 19, 2020

Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants