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

Change run.graphs and result.graphs from objects to arrays #326

Closed
michaelcfanning opened this issue Feb 20, 2019 · 6 comments
Closed

Change run.graphs and result.graphs from objects to arrays #326

michaelcfanning opened this issue Feb 20, 2019 · 6 comments
Labels
2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. e-ballot e-ballot-2 impact-breaks-consumers impact-breaks-producers merged Changes merged into provisional draft. resolved-fixed tc-32 tc-33 Issues to present at SARIF TC33

Comments

@michaelcfanning
Copy link
Contributor

No description provided.

@michaelcfanning
Copy link
Contributor Author

Consult with Semmle to verify whether we can move this 'future' and remove from v2.

@michaelcfanning
Copy link
Contributor Author

michaelcfanning commented Feb 27, 2019 •
edited by ghost
Loading

EBALLOT #1 PROPOSAL

NOTE Although it was approved, this proposal will not be implemented. After feedback from Semmle, we decided to keep graph support, but to represent it with arrays instead of dictionaries for consistency with other areas of the spec. See EBALLOT #2 PROPOSAL below.

The proposed change was to remove all graph-related constructs from the format, to close this issue for v2, and mark it 'future'. In a future revision of the format, we would reconsider adding this functionality (with some updates to bring its design into conformance with other patterns).

API IMPACT

remove 'edge'
remove 'edgeTraversal'
remove 'externalFileProperties.graphs'
remove 'graph'
remove 'graphTraversal'
remove 'node'
remove 'result.graphs'
remove 'result.graphTraversals'
remove 'run.graphs'

@michaelcfanning michaelcfanning changed the title Consider removing graphs and related Remove graphs and related types from v2 Feb 27, 2019
@michaelcfanning michaelcfanning added 2.1.0-CSD.1 Will be fixed in SARIF v2.1.0 CSD.1. e-ballot labels Feb 27, 2019
@harleenkohli harleenkohli mentioned this issue Mar 5, 2019
Closed
ghost pushed a commit that referenced this issue Mar 5, 2019
#325: Finish localization scrub; #326: toolComponent property changes.
f5e35af
@ghost ghost changed the title Remove graphs and related types from v2 Change run.graphs and result.graphs from objects to arrays Mar 7, 2019
@michaelcfanning michaelcfanning added the tc-33 Issues to present at SARIF TC33 label Mar 7, 2019
ghost pushed a commit that referenced this issue Mar 9, 2019
Change draft for #326: graph arrays.
e42c1f5
@ghost ghost self-assigned this Mar 14, 2019
ghost pushed a commit that referenced this issue Mar 15, 2019
Merge changes for #326: graphs arrays.
2eaf5ba
@ghost ghost added the merged Changes merged into provisional draft. label Mar 15, 2019
@ghost
Copy link

ghost commented Mar 16, 2019 •
edited by ghost
Loading

EBALLOT #2 PROPOSAL

After feedback from Semmle, we will retain graphs support in the format, but will represent the sets of graphs and graph traversals as arrays rather than dictionaries for consistency with the rest of the spec.

SCHEMA CHANGES

  • In the run object:

    • Change the type of the graphs property from object with graph-valued properties to graph[], unique, minItems: 0, default: empty array.

  • In the result object:

    • Change the type of the graphs property from object with graph-valued properties to graph[], unique, minItems: 0, default: empty array.
    • Change the type of the graphTraversals property from object with graphTraversal-valued properties to graphTraversal[], unique, minItems: 0, default: empty array.

  • In the graphTraversal object:

    • Remove the graphId property.
    • Add a property runGraphIndex, of type integer, optional, default: -1, minValue: -1
    • Add a property resultGraphIndex, of type integer, optional, default: -1, minValue: -1

  • In the externalProperties object:

    • Rename graph property to graphs and change its type from object with graph-valued properties to graph[], optional, unique, minItems: 0, default: empty array.

ghost pushed a commit that referenced this issue Mar 16, 2019
#326: Add missing schema change to schema changes document.
ac47a84
@kupsch
Copy link

kupsch commented Mar 19, 2019

The description in the OASIS SARIF ballot for this issue (https://www.oasis-open.org/apps/org/workgroup/sarif/ballot.php?id=3352) says that the "... change is to remove all graph-related constructs from the format ...". This is the text from #326 EBALLOT #1 PROPOSAL comment, but draft is for the #326 EBALLOT #2 PROPOSAL comment. Just wanted to confirm that the ballot text is incorrect and that we are voting on EBALLOT #2 PROPOSAL comment.

@ghost
Copy link

ghost commented Mar 19, 2019 •
edited by ghost
Loading

@kupsch Yes, the ballot text is incorrect and we are voting on the EBALLOT #2 PROPOSAL. I will inform the TC.

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.
@harleenkohli harleenkohli mentioned this issue Mar 21, 2019
Merged
@ghost
Copy link

ghost commented Mar 29, 2019

Approved in e-ballot #2.

@ghost ghost closed this as completed Apr 6, 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. e-ballot e-ballot-2 impact-breaks-consumers impact-breaks-producers merged Changes merged into provisional draft. resolved-fixed tc-32 tc-33 Issues to present at SARIF TC33
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@michaelcfanning @kupsch