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

implement AbstractFlashcardViewer / Javascript API versioning #6328

Closed
mikehardy opened this issue Jun 1, 2020 · 4 comments · Fixed by #6521
Closed

implement AbstractFlashcardViewer / Javascript API versioning #6328

mikehardy opened this issue Jun 1, 2020 · 4 comments · Fixed by #6521
Labels
API Enhancement Help Wanted Requesting Pull Requests from volunteers Keep Open avoids the stale bot

Comments

@mikehardy
Copy link
Member

Reproduction Steps
  1. alter the functionality in AnkiDroid's Javascript interface for a new version
  2. open a card with template based on advanced javascript functionality (e.g. Make available current card's and deck's details in WebView #6307)
Expected Result

Some indication that your card template isn't working correctly, and that there is a real reason, and ideally who to contact and maybe how to fix it yourself

Actual Result

Card might just break, no idea why

From @david-allison-1 in #6307 (review) I liked this idea a lot:

If we go down this route, we will want a method which will enable this complex functionality. Maybe not for this PR, but we should flesh out the plans, or we'll get hit by bad reviews that we can't do much about.

Something along the lines of: Define an "init" function, which accepts an API version number, and will toggle AbstractFlashcardViewer functions/signals in a forwards/backwards compatible manner.

So perhaps something like:

  • add an "ankidroidJsStatus" to AbstractFlashcardViewer
  • initialize that with contents of a 'ankidroidJsInit(int apiVersion, string developerContact)'
  • inject a warning in review below the reviewer minibar / top of card if any parts of the interface were used with either no init or a deprecated API, including a link to a wiki page for more info - bonus points if it is dismissable for the entirety of a review session
  • a wiki page explaining that we're adding to the interface and need to be able to version it to manage change over time, plus the exact changes (ideally copy-pastable for users) to get rid of the warning

Or similar, I'm more vague on the last points, anything reasonable to do versioning with user / developer notification of changes and ability to self help is welcome

@mikehardy
Copy link
Member Author

To be more clear:

  • the javascript should attempt to call 'ankidroidJSInit(string apiVersion, string developerContact' no matter what. That API may not exist on AnkiDroid yet so it should be done with exception handling.
  • the API version should start 1.0.0 and I suggest PRs that change the API should should alter the API version using semantic versioning

Concrete example of the versioning point:

You propose a PR that adds something the JS/WebView interface? That's a minor version bump in the API. (from 1.0.0 to 1.1.0)

You propose a PR that reorganizes the API completely or alters how the API reacts to the same inputs (a breaking change)? Major version bump: 1.0.0 becomes 2.0.0 - these should be done only when absolutely necessary. All effort should be made to be backwards-compatible instead via deprecation, ideally with warning in the Reviewer for users of the old API that there is a major version change coming that alters behavior they rely on, if they use the deprecated features

You propose a PR that just fixes a bug? patch version: 1.0.0 becomes 1.0.1

@github-actions
Copy link
Contributor

github-actions bot commented Aug 3, 2020

Hello 👋, this issue has been opened for more than 2 months with no activity on it. If the issue is still here, please keep in mind that we need community support and help to fix it! Just comment something like still searching for solutions and if you found one, please open a pull request! You have 7 days until this gets closed automatically

@github-actions github-actions bot added the Stale label Aug 3, 2020
@krmanik
Copy link
Member

krmanik commented Aug 3, 2020

Hello bot, api is in progress.

@david-allison david-allison added API Keep Open avoids the stale bot and removed Stale labels Aug 3, 2020
@mikehardy
Copy link
Member Author

Totally in progress and close I think! I was just on vacation but am getting back at it, I know this one is nearly ready but I haven't had time to review (sorry!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
API Enhancement Help Wanted Requesting Pull Requests from volunteers Keep Open avoids the stale bot
Projects
None yet
3 participants