Skip to content

Remove hashie gem#10491

Merged
zachmargolis merged 3 commits intomainfrom
margolis-remove-hashie
Apr 24, 2024
Merged

Remove hashie gem#10491
zachmargolis merged 3 commits intomainfrom
margolis-remove-hashie

Conversation

@zachmargolis
Copy link
Contributor

Turns out the one usage of hashie gem was inside Proofing::Aamva::Applicant. I don't think the gem is that critical, and can be replaced with standard library things, so giving this a shot!

Happy to clsoe if we don't need it

- Switch Proofing::Aamva::Applicant to be a Struct

changelog: Internal, Source code, Remove dependency on hashie gem
@zachmargolis zachmargolis requested a review from a team April 23, 2024 20:51
module Proofing
module Aamva
class Applicant < Hashie::Mash
Applicant = Struct.new(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another option is to use Data.define for read-only versions of these objects. However, the specs seemed to rely on hash key access, which structs provide, but data objects do not

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also open to using RedactedStruct if we think there's risk of these being copy-pasted in the console too

Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for RedactedStruct, on general paranioa principles, but I don't feel strongly about it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done in 9280eb4

Copy link
Contributor

@jmax-gsa jmax-gsa left a comment

Choose a reason for hiding this comment

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

LGTM

Thanks; it's nice to see a gem removed.

module Proofing
module Aamva
class Applicant < Hashie::Mash
Applicant = Struct.new(
Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for RedactedStruct, on general paranioa principles, but I don't feel strongly about it.

Copy link
Contributor

@mitchellhenke mitchellhenke left a comment

Choose a reason for hiding this comment

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

👏🏼

@zachmargolis zachmargolis merged commit b626eb5 into main Apr 24, 2024
@zachmargolis zachmargolis deleted the margolis-remove-hashie branch April 24, 2024 18:12
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.

3 participants