Skip to content
This repository has been archived by the owner on Sep 27, 2022. It is now read-only.

Minor changes #14

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Minor changes #14

wants to merge 5 commits into from

Conversation

djgamerr
Copy link
Contributor

*GetTorrent() now returns a single Torrent object
*GetProps() now return a single Props object
+GetList(bool) now has a optional parameter to fetch all props along with torrents
+Added TorrentNotFoundException

-Had to make changes to UnitTest, but cant test it since i dont have that installed.

Also, why is GitHub behaving as if the first merge never happened?

djgamerr added 5 commits June 3, 2019 20:19
*GetTorrent() now returns a single Torrent object
*GetProps() now return a single Props object
+GetList(bool) now has a optional parameter to fetch all props along with torrents
+Added TorrentNotFoundException

-Had to make changes to UnitTest, but cant test it since i dont have that installed.
@djgamerr
Copy link
Contributor Author

Now i noticed why, you added an enum to props in the meantime, you can just leave your props and parsesr

@latop2604
Copy link
Owner

I used squash and merge button. It's why you see only one commit until instead of many

@latop2604
Copy link
Owner

You should rebase your branch on master

@djgamerr
Copy link
Contributor Author

djgamerr commented Jun 12, 2019

Well, for now you can discard those two files and merge utorrentclientapi, units and new exception...
How to rebase?

@latop2604
Copy link
Owner

@latop2604
Copy link
Owner

latop2604 commented Jun 12, 2019

But I'm not sure to want this changes.
I prefer to keep client method as atomic as possible. Only one remote call each time.
The aggregation could be in an extension class.

For return type, you will completely hide the api error message/code that you should find in the Response object.

Theses contract changes will break compatibility with existing projects.

But I have to admit that I did this project a long time ago, and it's look really weird to me now on many aspects.

@djgamerr
Copy link
Contributor Author

I noticed that you had 2 methods doing the exact same things, errors could be handled as an exception, but thats true, it will break apps on update.
So i think its the best to keep it as a seperate fork?

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

Successfully merging this pull request may close these issues.

2 participants