search

Rayquaza01 / changelogger

A Firefox extension that gets changelogs for other Firefox extensions.
https://addons.mozilla.org/en-US/firefox/addon/changelogger/
MIT License
9 stars 1 forks source link

sync changelogger settings across devices #6

Closed kinghat closed 3 years ago

kinghat commented 3 years ago

would like to sync the settings across devices if you would accept of pr when i get around to it?

Rayquaza01 commented 3 years ago

Sure, I'll accept the PR if you make one

kinghat commented 3 years ago

i forgot to update the readme > options > theme documentation in the PRs

Rayquaza01 commented 3 years ago

That's fine, I can add that myself. The extension/docs/help.html also needs to be updated.

kinghat commented 3 years ago

The extension/docs/help.html also needs to be updated.

if you remove the theme section from this ill apply the styling/theme media queries to this as well, to bring it all together, later this afternoon or tomorrow.

Rayquaza01 commented 3 years ago

I ended up removing the help page and moving most of the unique points it had into the README. It mainly said the same things as the README anyway.

I also did a decent bit of work cleaning up the code (adding comments, fixing the race condition bug, updating the AMO API version, converting to TypeScript, etc.) on the feature branch. I think it's pretty much ready to go, but I'd be happy to hear if you would like to make any changes/suggestions before I submit it.

kinghat commented 3 years ago

oh neat, ive been learning the basics of TS recently. i will give it a look over and see what you've done. i appreciate you valuing my opinion on the project :pray:

kinghat commented 3 years ago

looked over the feature branch rewrite, it looks really clean and i learned some things. first time seeing a semaphore. is that the fix for the race condition bug? i tested it a bit and didnt find any issues. theres no easy way to see the sync storage area or test the sync api though :man_shrugging: thanks for your work on refactoring the project!

superficially "updated" here could be "shown": https://github.com/Rayquaza01/changelogger/blob/220187dec5e841b3b1b96baa19697bbeb23980c7/src/pages/options/options.html#L11 and the corresponding interface comment: https://github.com/Rayquaza01/changelogger/blob/220187dec5e841b3b1b96baa19697bbeb23980c7/src/OptionsInterface.ts#L2

Rayquaza01 commented 3 years ago

Yes, the semaphore is the fix for the race condition bug. Saving the changelog to storage is actually 3 smaller steps, reading the current list, updating the list, and writing the new list. Because the WebExtension storage API is async, if 2+ extensions were installed/updated at basically the same time, the steps might execute out of order, causing some of the changelogs to not actually be saved. The semaphore "locks" storage so that it's guaranteed that only one changelog can be written at once, and that any others trying to access storage will have to wait for the lock to be released.

Good catch on that comment, it makes more sense as "shown".

And thanks again for your help on this version! I really appreciate it!

Rayquaza01 commented 3 years ago

The new version should be up on AMO now. That was faster than expected! It only took 4 minutes for the submission to be approved.

kinghat commented 3 years ago

very nice. just came through for me and updated nicely. enjoy your week!