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 categories WebAPI #9586

Merged
merged 1 commit into from
Oct 12, 2018
Merged

Conversation

Piccirello
Copy link
Member

Closes #5330.

@glassez glassez added the WebAPI WebAPI-related issues/changes label Sep 23, 2018
src/webui/api/appcontroller.cpp Outdated Show resolved Hide resolved
src/webui/api/torrentscontroller.cpp Outdated Show resolved Hide resolved
@glassez glassez requested a review from Chocobo1 October 9, 2018 06:10
@glassez glassez merged commit 3e8f3d6 into qbittorrent:master Oct 12, 2018
@glassez
Copy link
Member

glassez commented Oct 12, 2018

Thank you!

@Piccirello
Copy link
Member Author

How should we track the API change resulting from this PR? Before release 4.1.4 is cut, the web API version will need to be bumped and the wiki updated. I know we discussed the idea previously.

@glassez
Copy link
Member

glassez commented Oct 17, 2018

web API version will need to be bumped

@sledgehammer999 do it before release.
We can help it by maintaining corresponding comment near API version definition and update it with each affected PR.

and the wiki updated.

I think the current approach (we just change wiki without any versioning) is incorrect. Users can use on different application versions providing different API versions.

@Piccirello
Copy link
Member Author

Maybe we can track the WebAPI changes in an issue while we’re between versions. I like that more than using the source code as a temporary log.

I think the current approach (we just change wiki without any versioning) is incorrect.

I’ve tried to partially address this by explicitly listing changes between API minor version in the Changes section.

@glassez
Copy link
Member

glassez commented Oct 17, 2018

Maybe we can track the WebAPI changes in an issue while we’re between versions.

I have no general objections.
What exactly should be in this Issue?

I’ve tried to partially address this by explicitly listing changes between API minor version in the Changes section.

It seems iineffective to me.
If user has no latest app version it still can't see appropriate api documentation. It just sees that something has changed since its version.

@glassez
Copy link
Member

glassez commented Oct 17, 2018

@Piccirello, shouldn't we change API version in first affecting PR? Currently we may have outdated version between releases. (We still can track it in corresponding Issue)

@Piccirello
Copy link
Member Author

Piccirello commented Oct 17, 2018

What exactly should be in this Issue?

Probably a list of PRs with webapi changes, as well as wiki text describing the changes.

It seems iineffective to me.

I agree, but I think it’s better than what we had before. There’s still room for improvement.

shouldn't we change API version in first affecting PR? Currently we may have outdated version between releases.

I think we should actually. One issue I can think of is where I have multiple concurrent PRs open: which should include the version bump? I guess if both PRs have it then git will handle it in the merge.

@Piccirello Piccirello deleted the webui-categories branch October 17, 2018 14:05
@glassez
Copy link
Member

glassez commented Oct 17, 2018

I think it’s better than what we had before.

I'm not suggesting you take a step back. On the contrary, we can (and should) do it even better. Add new things marked as "since v2.x.y" and don't remove the old ones but just mark it as "until v2.x.y". If we change format of some thing (like you done with "categories" value in "sync/maindata) we just have to mark old as "until..." and add new marked as "since..." (like on "cppreference.com").

@glassez
Copy link
Member

glassez commented Oct 17, 2018

One issue I can think of is where I have multiple concurrent PRs open: which should include the version bump? I guess if both PRs have it then git will handle it in the merge.

Each PR should be considered separately since you don't know what exactly will be merged first. Then you just rebase remaining PRs to resolve conflicts (as we do in case of any other conflicting PRs).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebAPI WebAPI-related issues/changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants