Proposed change to get_values behavior#1535

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Closed

kfreezen wants to merge1 commit intogitpython-developers:mainfromkfreezen:fix/get_values

Closed

Proposed change to get_values behavior#1535

kfreezen wants to merge1 commit intogitpython-developers:mainfromkfreezen:fix/get_values

Conversation

Copy link

Contributor

kfreezen commentedJan 10, 2023

get_values() did not load existing sections before checking the_sections property.

NOTE: Haven't written a test to check for this yet.

Copy link

ContributorAuthor

kfreezen commentedJan 10, 2023

Proposed fix for#1534

Copy link

ContributorAuthor

kfreezen commentedJan 10, 2023

Looking at test here; I suppose we can leave it raising a KeyError. The main fix was force-loading the_sections attribute before checking the list.

I'll re-commit using KeyError.

get_values eagerly loads sections before return

cfc613a

kfreezen force-pushed thefix/get_values branch fromc18b319 tocfc613aCompare

January 10, 2023 19:17

Byron added this to thev3.1.31 - Bugfixes milestone

Jan 11, 2023

Byron requested changes

Jan 11, 2023

View reviewed changes

Copy link

Member

Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot, much appreciated!

I think the commit should link back to the issue it resolve to create a more declarative history.
Finally it should be possible to devise a test that would fail without this change to cement it. Otherwise it might be removed in future because it's unclear why eager loading of sections is necessary.