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

Fixed the scope issue for the private method #167

Merged
merged 2 commits into from
Oct 24, 2023
Merged

Conversation

ashwinisukale
Copy link
Contributor

@ashwinisukale ashwinisukale commented Oct 20, 2023

Purpose

closes: #168

Approach

Upgrade bolognese gem version in Lupo from 1.11.0 to 1.11.2.
abstract_description is a helper method created to show abstract description, this is not a field in database. When we upgraded there is update in zeitwerk gem from 2.6.1 to 2.6.8

So the way class loads might have become strict, hence it was working before and now it's failing.

Open Questions and Pre-Merge TODOs

Learning

Types of changes

  • Bug fix (non-breaking change which fixes an issue)

  • New feature (non-breaking change which adds functionality)

  • Breaking change (fix or feature that would cause existing functionality to change)

Reviewer, please remember our guidelines:

  • Be humble in the language and feedback you give, ask don't tell.
  • Consider using positive language as opposed to neutral when offering feedback. This is to avoid the negative bias that can occur with neutral language appearing negative.
  • Offer suggestions on how to improve code e.g. simplification or expanding clarity.
  • Ensure you give reasons for the changes you are proposing.

@ashwinisukale ashwinisukale requested a review from a team October 20, 2023 16:14
@ashwinisukale ashwinisukale self-assigned this Oct 20, 2023
def abstract_description
# Fetch the first description with descriptionType "Abstract"
descriptions&.find { |d| d["descriptionType"] == "Abstract" }
end
Copy link
Contributor

Choose a reason for hiding this comment

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

i'm just wondering if we could perhaps DRY this implementation since we do repeat this method. I'm not sure about the code structure but maybe it could go into a writers concern or in some utility module? if this is not possible please do not hesitate to let me know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are absolutely correct Wendel, I am just thinking what would be best place to add these to helper file. Thank you for the review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wendelfabianchinsamy moved the method to the common place.

Copy link
Contributor

Choose a reason for hiding this comment

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

Brilliant! Nice one @ashwinisukale 🚀.

Copy link
Contributor

@svogt0511 svogt0511 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@ashwinisukale ashwinisukale merged commit 9c765ab into master Oct 24, 2023
5 checks passed
@ashwinisukale ashwinisukale deleted the bug/upgrade_issue branch October 24, 2023 11:07
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.

Upgrading the gem gives error undefined abstract_description method for DataciteDoi
3 participants