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 Author field #21

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

Add Author field #21

wants to merge 3 commits into from

Conversation

cashpw
Copy link
Contributor

@cashpw cashpw commented Oct 11, 2020

Resolves #20.

@cashpw
Copy link
Contributor Author

cashpw commented Oct 11, 2020

It looks like I didn't set the upgrade path correctly. I'm not seeing the "In order to import..." dialog when starting Anki.

@sobjornstad
Copy link
Owner

sobjornstad commented Oct 11, 2020

Ah, I think you've misunderstood how the note type classes work. For a minor upgrade like this one where the change can be made in place without creating a new note type, you don't need to create a new class -- you update the existing class to be the version that a new user would get, then make the upgrade_* routine apply the changes necessary to get the previous version to match the new version.

@sobjornstad
Copy link
Owner

sobjornstad commented Oct 11, 2020

And you need to add an item to the upgrades tuple on the LpcgOne note type to get LPCG to notice that there's a new upgrade available. Details documented in the upgrade_from() method of the ModelData superclass.

@cashpw cashpw force-pushed the master branch 2 times, most recently from 2fdcd63 to 5457919 Compare October 11, 2020 14:25
@cashpw
Copy link
Contributor Author

cashpw commented Oct 11, 2020

There; I think I've fixed it up. I tested it locally (at 1.3.0, then upgrading to 1.3.1 by copying the built files into the addon directory) and everything worked.

Copy link
Owner

@sobjornstad sobjornstad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this in! Should be a nice addition.

src/models.py Outdated Show resolved Hide resolved
src/models.py Outdated Show resolved Hide resolved
designer/import_dialog.ui Outdated Show resolved Hide resolved
designer/import_dialog.ui Outdated Show resolved Hide resolved
designer/import_dialog.ui Outdated Show resolved Hide resolved
@sobjornstad
Copy link
Owner

@cashweaver Are you going to have a chance to take a look at these requested tweaks soon? I'd like to get a new version out.

@cashpw
Copy link
Contributor Author

cashpw commented Dec 16, 2020

@sobjornstad Sorry for the delay!

sobjornstad pushed a commit that referenced this pull request Jun 6, 2021
Closes #20 and PR #21.
sobjornstad pushed a commit that referenced this pull request Jun 6, 2021
Closes #20 and PR #21.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add an author field
2 participants