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

Bk/refactor/base entities #38

Merged
merged 3 commits into from
Mar 19, 2024
Merged

Conversation

babslgam
Copy link
Contributor

@koeaw: pull request should be small enough. please report back, if not

use new fields provided by datasource model, remove old ones
use single data source object for all base entities, use correct use fields for data source and property model
@koeaw
Copy link
Collaborator

koeaw commented Mar 19, 2024

Great that there is another PR! I wanted to try to run the script but realised the Zotero variables were still missing, so would have had to manually force a redeployment...

@koeaw
Copy link
Collaborator

koeaw commented Mar 19, 2024

PR size is so much better, thx!

Constructive criticism re: the file deletion commit – in a case like this where a file is replaced/becomes redundant, the deletion is ideally committed simultaneously with the replacement file/changes. For misc. removals of redundant, leftover, old stuff n cruft, I'd be very direct and to the point about that in the summary/subject line and put explanations or further details in the commit body instead.

I'll merge these as-is now as I noticed the two deployment branches have diverged, so deploy_imports will need some manually updating anyway to get the latest commits on deploy.

@koeaw koeaw merged commit 3402d26 into deploy_imports Mar 19, 2024
1 check passed
@babslgam babslgam deleted the bk/refactor/base-entities branch April 5, 2024 14:28
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.

None yet

2 participants