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

Fix issue #283: Rename Key Group to Entity Set #329

Merged
merged 7 commits into from
Oct 10, 2024

Conversation

tientong98
Copy link
Contributor

@tientong98 tientong98 commented Oct 4, 2024

Closes #283. <-- Taylor: put the issue you're addressing here so merging the PR will close the issue

Changes proposed in this pull request

  • Use re to change all instances of Key Group to Entity Set (matching capitalization and delimiter) across all files that can be decoded using UTF-8.
  • Update to Ubuntu 2404:2024.08.1 and [email protected] to pass CircleCI test (from 1.14.9 to 1.14.13, there is an issue with JSON schema validation), change build_validator_call func in cubids/validator.py to follow the new syntax (require this version somewhere in the doc?)

@tientong98
Copy link
Contributor Author

Hi @tsalo and @mattcieslak - circleci failed because it used ubuntu-2004:202201-02, which is now deprecated and not available . I want to change the config.yml file so that it uses one of the current images, but I'm not sure if it will break anything else. Do you have any preference on which image to use?

@mattcieslak
Copy link
Contributor

Any of those should be ok

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

Just one requested change. Also, this is a breaking change, so I'd add a breaking-change label to the PR and we need to make sure the next release is a major one.

HISTORY.rst Outdated Show resolved Hide resolved
@tientong98 tientong98 requested a review from tsalo October 9, 2024 15:54
Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@mattcieslak mattcieslak left a comment

Choose a reason for hiding this comment

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

Excellent!! I like the new name much better

docs/usage.rst Outdated Show resolved Hide resolved
Co-authored-by: Matt Cieslak <[email protected]>
@tientong98 tientong98 merged commit 065444d into PennLINC:main Oct 10, 2024
8 checks passed
@tientong98 tientong98 deleted the fix-issue-283 branch October 31, 2024 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rename Key Group to Entity Set
3 participants