search

FooSoft / yomichan

Japanese pop-up dictionary extension for Chrome and Firefox.
https://foosoft.net/projects/yomichan
Other
1.06k stars 218 forks source link

Errors in glossary and furigana field of generated anki note [20.6.27.0] #644

Closed Pintliz closed 4 years ago

Pintliz commented 4 years ago

image missing texts and [object Object] in glossary field

image all entries in furigana field becomes undefined

ankiconnect ver: (updated 2020-06-26)

toasted-nutbread commented 4 years ago

Fixed in #645. @FooSoft: patch release tag 20.6.27.1 created. Not sure what the workflow should be since the release is still in testing: a new date version for the release, or just the patch.

FooSoft commented 4 years ago

I think that we should just have a new version without hotpatch bit. If a version is bad and gets caught in testing, we can just retire it I think.

toasted-nutbread commented 4 years ago

Okay; the new commits added since the release are: 0a6c08d0f53090a4ad48663bc5846ddae5723d52...bc6d855f3dd7a8ac198593c15a25dca53a89a18f. bc6d855f3dd7a8ac198593c15a25dca53a89a18f is the fix. It's probably be fine to include them all and update the release branch, or to just re-tag/update the version of 20.6.27.1 and treat it as the release branch. The workflow of continuing to commit to master during a testing release is a bit odd due to this, since there are commits going into master which weren't intended for the release (i.e. may not be fully tested), and I'm not sure if it's an issue if the release branch temporarily diverges from master.

FooSoft commented 4 years ago

That makes sense -- maybe we should go with 20.6.27.1 after all then. I can see a degenerate situation where no testing releases are promoted to release because of bugs being found for several iterations. From that point of view, it definitely makes sense to consider this being a hotpatch. I'll cherry pick the change over to testing and tag it there. Not sure where it would make sense to tag hotfix change as a general rule though, since it is possible that a hotfix comes after unrelated changes.

FooSoft commented 4 years ago

So here is a thought -- we don't have hotfix tags in master, since it can be confusing to describe the code necessary to fix testing in terms of master changes. I would imagine that we might even have situations where we have a "quick fix" for testing which we would not even want to have in master. Also semantically, we are hotpatching testing and not master...

Thoughts? This approach may require a release branch to which testing flows. My approach up until now was just to checkout the tag of the testing version and build the deployment zips from there. That might not be robust enough to deal with all hotfix scenarios.

toasted-nutbread commented 4 years ago

Not sure where it would make sense to tag hotfix change as a general rule though, since it is possible that a hotfix comes after unrelated changes.

The hotfix tags I've made so far have basically been branches off of the release branch with the fixing commits cherry-picked onto it. So the hotfix commits don't contain anything unrelated to that release. Compare: master history @ bc6d855f3dd7a8ac198593c15a25dca53a89a18f vs 20.6.27.1 history @ 9e15f84733a482cea0104d78617dd49699755ae7. This is the divergence I was mentioning.

I've marked these using tags in the past to avoid having branch clutter, but another potential workflow would be to mark release branches as something like release/20.6.27, and if any hotfixes are necessary (during either testing or stable), they can just be cherry-picked from master onto that release (backported if necessary). Since the commits all have PR identifiers in them, this makes it easy to correlate the divergences.

This may affect the workflow for generating releases, since the testing branch has slightly different metadata for the manifest and whatnot. So the build process may be slightly different, such as just having a script which can be used like build testing or build stable, and it auto-generates a temp manifest before zipping. Longer term, the scope of this building issue goes beyond more than just testing/stable releases. I mentioned it a bit in #612, where there may need to be a slight difference in the manifest used for Chrome vs the one used for Firefox due to certain browser-features.