Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix for #2087 #2147

Merged
merged 2 commits into from
Jan 25, 2024
Merged

Fix for #2087 #2147

merged 2 commits into from
Jan 25, 2024

Conversation

a-sr
Copy link
Collaborator

@a-sr a-sr commented Jan 10, 2024

Fixes incorrect scope for detection of duplicate IDs in attribute parameters. Now only duplicate parameter IDs in the same attribute raise and error.
Closes #2087.

@a-sr a-sr added the bugfix label Jan 10, 2024
@a-sr a-sr requested a review from lhstrh January 10, 2024 14:00
@a-sr a-sr force-pushed the fix-duplicate-attrparm branch from a902482 to d4bab05 Compare January 10, 2024 14:10
Copy link
Member

@lhstrh lhstrh 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! This should be covered by a unit test, however; one that expects no errors when the same attribute name is used in two different locations and one that expects an error for code that indeed has duplicate attributes.

@lhstrh lhstrh self-requested a review January 24, 2024 23:45
@lhstrh lhstrh force-pushed the fix-duplicate-attrparm branch from 678c6bb to df85bbf Compare January 24, 2024 23:46
@lhstrh lhstrh enabled auto-merge January 24, 2024 23:47
@lhstrh lhstrh disabled auto-merge January 25, 2024 00:30
@lhstrh lhstrh merged commit 3177bc1 into master Jan 25, 2024
28 of 43 checks passed
@lhstrh lhstrh deleted the fix-duplicate-attrparm branch January 25, 2024 00:30
@a-sr
Copy link
Collaborator Author

a-sr commented Jan 25, 2024

Thank you for adding the test. I was quite busy the last days.

@lhstrh
Copy link
Member

lhstrh commented Jan 25, 2024

No worries. I realized, however, that we still have some work to do in terms of validation, but that can be done later (see #2162).

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distinct labels flagged as duplicate
2 participants