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

Update the message for undefined scrub #36

Merged
merged 3 commits into from
Jun 13, 2024
Merged

Conversation

philton4om
Copy link
Contributor

@philton4om philton4om commented Jun 13, 2024

Update the warning message to delineate the class vs the field.

Example of the old output which had no delination between the class and field:

Undefined scrub: aspen_reconciliations_fhir_resource_json for InboundFhirItemresource

New change would output

Undefined scrub: aspen_reconciliations_fhir_resource_json for InboundFhirItem.resource

@philton4om philton4om requested a review from adamstegman June 13, 2024 18:22
Copy link
Member

@adamstegman adamstegman left a comment

Choose a reason for hiding this comment

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

Can you also bump the version? This seems like a patch version to me 🙂

@@ -1,3 +1,3 @@
module ActsAsScrubbable
VERSION = '2.1.3'
VERSION = '2.1.4'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adamstegman Updated this

But for a more general question - are the workflows proven out to work? I have been unable to identify the cause of the id error.

It works locally (I hate making that statement) with ruby 3.2.4.

@philton4om ➜ /workspaces/acts_as_scrubbable (no-jira/philton4om-patch-1) $ rspec
Resolving dependencies...
2024-06-13 20:06:11 +0000: [INFO] - Using Upsert
.2024-06-13 20:06:11 +0000: [INFO] - Using Update
.2024-06-13 20:06:11 +0000: [INFO] - Using Update
2024-06-13 20:06:11 +0000: [INFO] - Scrubbing ScrubbableModel ...
2024-06-13 20:06:11 +0000: [INFO] -  ScrubbableModel objects scrubbed
2024-06-13 20:06:11 +0000: [INFO] - Scrub Complete!
.2024-06-13 20:06:11 +0000: [INFO] - Using Update
2024-06-13 20:06:11 +0000: [INFO] - Scrubbing ScrubbableModel ...
2024-06-13 20:06:11 +0000: [INFO] -  ScrubbableModel objects scrubbed
2024-06-13 20:06:11 +0000: [INFO] - Scrub Complete!
.......................

Finished in 0.95615 seconds (files took 1.08 seconds to load)
26 examples, 0 failures

Copy link
Member

Choose a reason for hiding this comment

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

You can see the past builds in CircleCI: https://app.circleci.com/pipelines/github/onemedical/acts_as_scrubbable?branch=main. It hasn't been run in 5 months, so something upstream likely changed.

Copy link
Member

Choose a reason for hiding this comment

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

I take it back—a new branch with no changes passes. So there's something in here it doesn't like. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

You can reproduce it using the same Rails version as one of the failed jobs:

rbenv shell 3.2.2
export RAILS_VERSION_PREFIX=7.0
bundle install
bundle exec rspec

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Magic! Thank you - was unable to figure out how to do that. I reproduced it and will see what I can do.

Copy link
Member

Choose a reason for hiding this comment

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

Fixed in 0021ffd! This should be ready to merge (you can ignore the test-head failure, it's a known issue with nulldb).

@philton4om philton4om merged commit 6f1f6cb into main Jun 13, 2024
1 of 2 checks passed
@philton4om philton4om deleted the no-jira/philton4om-patch-1 branch June 13, 2024 22:09
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.

2 participants