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.

Sign up for GitHub

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

Jump to bottom

External property file related renames #335

Closed
ghost opened this issue Feb 27, 2019 · 3 comments
Closed

External property file related renames #335

ghost opened this issue Feb 27, 2019 · 3 comments
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. bug design-approved The TC approved the design and I can write the change draft e-ballot e-ballot-2 impact-breaks-consumers impact-breaks-producers merged Changes merged into provisional draft. p1 Priority 1 issue to close resolved-fixed sdk-code-complete tc-33 Issues to present at SARIF TC33

Comments

@ghost
Copy link

ghost commented Feb 27, 2019 •
edited by ghost
Loading

EBALLOT PROPOSAL: rename the external property files object 'properties' member so that it does not collide with the general SARIF design standard of populating all object types with a property bag of that name. This allows the external property file to both have its own property bag (named 'properties') while also allowing it to externalize a run-level properties object (which we will not call 'externalizedProperties')

API IMPACT
In the externalPropertyFiles object:

  • Rename the properties property to externalizedProperties.
  • Restore the properties property, which now refers to the object level property bag, not to externalized content.

In the externalPropertyFiles object:

  • Rename the instanceGuid property to guid.
  • Rename the artifactLocationproperty to location

POST-BALLOT NOTE
There is a typo in the ballot text. It should read "...which we will now call 'externalizedProperties'" instead of "... which we will not call 'externalizedProperties'".
@ghost ghost added bug impact-breaks-consumers impact-breaks-producers 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. p1 Priority 1 issue to close labels Feb 27, 2019
@michaelcfanning michaelcfanning changed the title Rename externalPropertyFiles.properties to externalizedProperties Rename ‘externalPropertyFiles.properties’ to ‘externalPropertyFiles.externalizedProperties’ Feb 27, 2019
@michaelcfanning michaelcfanning changed the title Rename ‘externalPropertyFiles.properties’ to ‘externalPropertyFiles.externalizedProperties’ External property file related renames Mar 1, 2019
ghost pushed a commit that referenced this issue Mar 5, 2019
Change draft for #335: externalPropertyFiles property changes.
1fd7480
@kupsch
Copy link

kupsch commented Mar 7, 2019

The object names in the "In the externalPropertyFiles object:" are incorrect: The first one should be "externalProperties" and the second one should be "externalPropertyFile". It would be more appropriate if these names are swapped: the object in the external file should be the "externalPropertyFile" object and the object in theRun.externalPropertyFiles[] should be "externalProperty" or better yet something like "externalPropertyReference".

If the instanceGuid property name is going to change to "guid" it should change in both objects for consistency.

@michaelcfanning michaelcfanning added the tc-33 Issues to present at SARIF TC33 label Mar 8, 2019
michaelcfanning pushed a commit to microsoft/sarif-sdk that referenced this issue Mar 8, 2019
@harleenkohli @michaelcfanning
335 updates to externalPropertyFiles [oasis-tcs/sarif-spec#335] (#1328)
c7831d8 
* code builds

* adding UTCs & transformer logic

* fixing post merge issues manually

* rc++
ghost pushed a commit that referenced this issue Mar 8, 2019
Rename draft for #335 to include #321.
7f963bf
ghost pushed a commit that referenced this issue Mar 8, 2019
Partial change draft for #321 and #335
8c7e96b 
This change draft will ultimately contain the changes for issues #321
(inlining externalized properties) and #335 (external property-file
renames), including all post-ballot changes.

Currently, it contains only the changes for #335.
@ghost
Copy link
Author

ghost commented Mar 8, 2019

Feedback from MS (post-ballot):

  • In the externalProperties object:
    • Rename the instanceGuid property to guid.
    • Rename the properties property to externalizedProperties.
    • Add a property named properties that is a property bag.
  • In the externalPropertyFile object:
    • Rename the object to externalPropertyFileReference.
    • Rename the instanceGuid property to guid.
    • Rename the artifactLocation property property to location.
  • With regard to the run.externalPropertyFiles property, and the object it contains:
    • Rename it to run.externalPropertyFileReferences.
    • Change the name of the contained object’s properties property to externalizedProperties.
    • Remove the run. prefix from the property names.

ghost pushed a commit that referenced this issue Mar 9, 2019
Finish combined change draft for #321 and #335.
10c358f
@ghost ghost mentioned this issue Mar 13, 2019
Closed
@ghost ghost self-assigned this Mar 14, 2019
@ghost ghost added the design-approved The TC approved the design and I can write the change draft label Mar 14, 2019
ghost pushed a commit that referenced this issue Mar 14, 2019
Merge changes for #321, #335: inlined external properties and name ch…
1678f6f 
…anges.
@ghost ghost added the merged Changes merged into provisional draft. label Mar 14, 2019
ghost pushed a commit that referenced this issue Mar 19, 2019
Provide "e-ballot #2" draft.
412c245 
This draft contains all the changes through "e-ballot #2," which
opened on Friday March 15 and will close on Friday March 22. It contains
changes for ballot issues #168, #291, #309, #320, #321, #326, #335,
and #341, as well as for previously approved issue #340.

It does _not_ contain changes for any issues from "e-ballot #3,"
which will open on Friday March 22 and close on Friday March 29.
@ghost
Copy link
Author

ghost commented Mar 29, 2019

Approved in e-ballot #2.

@ghost ghost closed this as completed Mar 29, 2019
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. bug design-approved The TC approved the design and I can write the change draft e-ballot e-ballot-2 impact-breaks-consumers impact-breaks-producers merged Changes merged into provisional draft. p1 Priority 1 issue to close resolved-fixed sdk-code-complete tc-33 Issues to present at SARIF TC33
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@michaelcfanning @kupsch