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

Jump to bottom

Definitions #304

Merged
merged 22 commits into from Jan 14, 2020
Merged

Definitions #304

merged 22 commits into from Jan 14, 2020

Conversation

TheFoxAtWork
Copy link
Collaborator

Checked with @SantiagoTorres looks good so far

@TheFoxAtWork TheFoxAtWork mentioned this pull request Dec 5, 2019
Closed
lumjjb
lumjjb reviewed Dec 11, 2019
View reviewed changes
Copy link
Collaborator

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

Thanks for creating this. Content is great! I've added a couple comments just around wording and nits.

supply-chain-security/compromises/compromise-definitions.md Outdated Show resolved Hide resolved
supply-chain-security/compromises/compromise-definitions.md Outdated Show resolved Hide resolved
supply-chain-security/compromises/compromise-definitions.md Outdated Show resolved Hide resolved
supply-chain-security/compromises/compromise-definitions.md Outdated Show resolved Hide resolved
supply-chain-security/compromises/compromise-definitions.md Show resolved Hide resolved
TheFoxAtWork
TheFoxAtWork commented Dec 16, 2019
View reviewed changes
Copy link
Collaborator Author

@TheFoxAtWork TheFoxAtWork left a comment

Choose a reason for hiding this comment

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

changes requested except one have been made

@TheFoxAtWork
Copy link
Collaborator Author

@lumjjb ready for ur review....

lumjjb
lumjjb reviewed Dec 17, 2019
View reviewed changes
Copy link
Collaborator

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

Replied to some of the changes - I think it looks good to me after the minor changes. If @SantiagoTorres can do a review of the deltas and LGTM as well. And let's add this on the agenda of one of the meetings to share this new doc as well!

supply-chain-security/compromises/compromise-definitions.md Outdated Show resolved Hide resolved
@SantiagoTorres
Copy link
Contributor

I like this, although I'm also not fully convinced if the word defeat is the term we'd like. In one hand I like that it does carries some strength/determination, but on the other I think it's not entirely clear what it means at first glance...

Other than that, pretty solid work @TheFoxAtWork !

TheFoxAtWork
TheFoxAtWork commented Dec 19, 2019
View reviewed changes
Copy link
Collaborator Author

@TheFoxAtWork TheFoxAtWork left a comment

Choose a reason for hiding this comment

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

pushed changes requests please review @lumjjb @SantiagoTorres

lumjjb
lumjjb approved these changes Dec 19, 2019
View reviewed changes
Copy link
Collaborator

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

LGTM

@SantiagoTorres
Copy link
Contributor

I can't approve, but LGTM as well :)

@lumjjb
Copy link
Collaborator

lumjjb commented Dec 19, 2019

@ultrasaurus @pragashj @dshaw This looks good to merge.. any additional comments?

@lumjjb
Copy link
Collaborator

lumjjb commented Jan 3, 2020

Just waiting for additional comments from co-chairs. can discuss this at next meeting and merge this probably

@dshaw dshaw self-requested a review January 7, 2020 03:25
dshaw
dshaw approved these changes Jan 7, 2020
View reviewed changes
Copy link
Collaborator

@dshaw dshaw left a comment

Choose a reason for hiding this comment

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

Looks good.

@ultrasaurus
Copy link
Member

This is fabulous. Just one formatting issue before merging -- pls make the sections in the doc be the same order as the index. Looks like that is alphabetical, which seems like a fine approach to me.

@TheFoxAtWork
Copy link
Collaborator Author

@ultrasaurus made the changes u requested. @SantiagoTorres also added in the link to the definitions on the readme under compromises

@ultrasaurus
Copy link
Member

Looks great -- thanks @TheFoxAtWork !

@ultrasaurus ultrasaurus merged commit 8352673 into cncf:master Jan 14, 2020
@TheFoxAtWork TheFoxAtWork deleted the definitions branch January 23, 2020 13:40
Michael-Susu12138 pushed a commit to Michael-Susu12138/tag-security that referenced this pull request Dec 12, 2023
@TheFoxAtWork @lumjjb
@dshaw @ultrasaurus
Supply Chain Compromise Definitions (cncf#304)
13b9fd3 
Co-authored-by: Brandon Lum <lumjjb@gmail.com>
Co-authored-by: Dan Shaw <github@dshaw.com>
Co-authored-by: Sarah Allen <sarah@ultrasaurus.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dshaw dshaw dshaw approved these changes

@lumjjb lumjjb lumjjb approved these changes

Assignees

@TheFoxAtWork TheFoxAtWork

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@TheFoxAtWork @SantiagoTorres @lumjjb @ultrasaurus @dshaw