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

Change name of 'standard' property to be 'standardName' #631

Merged
merged 2 commits into from
Feb 15, 2024

Conversation

sbarnum
Copy link
Collaborator

@sbarnum sbarnum commented Feb 6, 2024

This PR implements the consensus decision of the Feb 06, 2024 Tech call to address immediate concerns under Issue #387.

More structured approaches to fully resolving Issue #387 will be pursued post RC2.

@sbarnum sbarnum mentioned this pull request Feb 6, 2024
Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

I agree with the solution as well - and the PR LGTM.

Thanks @sbarnum and the participants on the tech team call.

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

I just noticed that the CI is failing - there is a class referencing standard that also needs updating. I believe it is Artifact.

@sbarnum - if you could add that change in as well.

@goneall
Copy link
Member

goneall commented Feb 6, 2024

I just noticed that the CI is failing - there is a class referencing standard that also needs updating. I believe it is Artifact.

@sbarnum - if you could add that change in as well.

I just confirmed it is in the Core/Artifact class - we just need to rename standard to standardName. @sbarnum - I can also make the changes to the branch if you prefer.

@kestewart kestewart added this to the 3.0-rc2 milestone Feb 7, 2024
@kestewart kestewart added the Profile:Core Core Profile and related matters label Feb 7, 2024
@maxhbr
Copy link
Member

maxhbr commented Feb 13, 2024

this fails with AssertionError: Property name standard does not match metadata standardName

@maxhbr
Copy link
Member

maxhbr commented Feb 13, 2024

this also needs to be changed in:

  • the title of the file
  • the places where it is used, e.g. in .../Core/Classes/Artifact.md
    • should the standard property in Artifact be using this?

@goneall
Copy link
Member

goneall commented Feb 13, 2024

@sbarnum @maxhbr - Let me know if you'd like me to update the PR with the required changes

@kestewart
Copy link
Contributor

@goneall - discussed this in AI call, and we're good with it as is. For future, Karen volunteers to coordinate with IEEE/ISO for future standard fields.

Copy link
Member

@goneall goneall left a comment

Choose a reason for hiding this comment

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

LGTM - CI failures are now fixed

Copy link
Contributor

@kestewart kestewart left a comment

Choose a reason for hiding this comment

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

Thanks Gary for fixing the remaining refs.

@kestewart kestewart merged commit 14e4662 into main Feb 15, 2024
1 check passed
@bact bact deleted the change-standard-property-to-standardName branch August 28, 2024 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Profile:Core Core Profile and related matters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants