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

Replace Preneeds Virtus Models with POROs #18494

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

Conversation

stevenjcumming
Copy link
Contributor

@stevenjcumming stevenjcumming commented Sep 18, 2024

Summary

  • Update Preneeds::Base to remove virtus
    • set all Preneeds models back to Preneeds::Base
  • re-implement Virtus's basic features in Preneeds::Base
    • this should have a minimal impact on the subclasses
  • spec/requests/v0/preneeds/burial_forms_spec.rb had a mistake when instantiating Preneeds::BurialForm and I fixed that

Related issue(s)

Testing done

  • n/a

Acceptance criteria

  • Preneeds::Base doesn't use virtus gem
  • Preneeds::VirtusBase is removed

Reviewer Feedback

A class instance variable (@attributes) is used to aggregate the attributes listed in the subclass. Rubocop suggested referencing it in a class method this could be a thread safety concern.

However, I think because the value of @attributes is based on the subclass implementation and therefore the same for ever thread there shouldn't be a thread safety concern. @attributes is effectively immutable because it's based on the the attributes listed in the file.

@stevenjcumming stevenjcumming self-assigned this Sep 18, 2024
@stevenjcumming stevenjcumming changed the base branch from master to sjc-preneeds-virtus-base September 18, 2024 13:27
@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file:

@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) was deleted but its reference still exists in CODEOWNERS. Please update the .github/CODEOWNERS file and delete the entry for the Offending file:

@va-vfs-bot va-vfs-bot temporarily deployed to sjc-preneeds-virtus-models/main/main September 18, 2024 14:03 Inactive
Base automatically changed from sjc-preneeds-virtus-base to master September 18, 2024 19:57
@va-vsp-bot
Copy link
Collaborator

Error: A file (or its parent directories) does not have a CODEOWNERS entry. Please update the .github/CODEOWNERS file and add the entry for the Offending file: config/initializers/core_extensions.rb

Copy link

Backend-review-group approval confirmed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants