Skip to content

Implement WriteIndex for cluster summaries#70

Merged
jetstack-bot merged 3 commits intojetstack:masterfrom
charlieegan3:write-report-index
Jan 30, 2020
Merged

Implement WriteIndex for cluster summaries#70
jetstack-bot merged 3 commits intojetstack:masterfrom
charlieegan3:write-report-index

Conversation

@charlieegan3
Copy link
Contributor

part of #54

@jetstack-bot jetstack-bot added the dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. label Jan 29, 2020
@jetstack-bot jetstack-bot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jan 29, 2020
Copy link
Member

@j-fuentes j-fuentes left a comment

Choose a reason for hiding this comment

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

modulo my comments on the azblob output.

}

// build a report to build the updated context for the report index
report, err := reports.NewReport(manifest, rc)
Copy link
Member

Choose a reason for hiding this comment

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

Possible optimization: We don't really need the whole report here. We could create here the ReportSummary.

But that's is not a reason to block this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah... I did wonder about this... it's not ideal.

@jetstack-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: charlieegan3, j-fuentes

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [charlieegan3,j-fuentes]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@jetstack-bot jetstack-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 30, 2020
@j-fuentes
Copy link
Member

/lgtm

@jetstack-bot jetstack-bot added lgtm Indicates that a PR is ready to be merged. and removed lgtm Indicates that a PR is ready to be merged. labels Jan 30, 2020
@charlieegan3
Copy link
Contributor Author

I had to rebase and fix a failed test...

Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
Signed-off-by: Charlie Egan <charlieegan3@users.noreply.github.com>
@j-fuentes
Copy link
Member

/lgtm

@jetstack-bot jetstack-bot added the lgtm Indicates that a PR is ready to be merged. label Jan 30, 2020
@jetstack-bot jetstack-bot merged commit b4872af into jetstack:master Jan 30, 2020
@charlieegan3 charlieegan3 deleted the write-report-index branch January 30, 2020 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates that all commits in the pull request have the valid DCO sign-off message. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants