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

Adds clarity to build model terminology (issue #714) #876

Merged
merged 8 commits into from
Jun 26, 2023

Conversation

chtiangg
Copy link
Contributor

@chtiangg chtiangg commented Jun 7, 2023

In the build model section:

  1. Define "Build Caches".
  2. Adds a collapse card for "Ambiguous terms to avoid", and clarify "build recipe" and "builder".
  3. Makes "Ambiguous terms to avoid" a collapse card in the Package model section.

After this change for the terms mentioned in issue #714:

  • Build process: is defined in "Build" before this PR, see link.
  • Build: is defined before this PR.
  • Build service: is defined in "Platform" before this PR, see link .
  • Builder: is clarified in "Ambiguous terms to avoid".
  • Build system: is defined in "Platform" before this PR, see link
  • Build environment: s defined before this PR.
  • Build tools: is included in "Dependencies" before this PR, see link
  • Build model: no need to define.
  • Build platform: is defined before this PR.
  • Build recipe: is clarified in "Ambiguous terms to avoid".
  • Build infrastructure: didn't change, but please let me know if it needs to be clarified.
  • Build configuration source: is clarified in "Ambiguous terms to avoid".
  • Build worker: didn't change, but not sure if this is really needed.
  • Build cache: is newly added to the term table.
  • Builder model: didn't change, but not sure if this is really needed.
  • Build executor: is somewhat defined in "Build environment" before this PR, see link
  • Build definition: is clarified in "Ambiguous terms to avoid".
  • Control Plane: is defined before this PR.

@netlify
Copy link

netlify bot commented Jun 7, 2023

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit 4956414
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/6499b5696d43bf00087a1af9
😎 Deploy Preview https://deploy-preview-876--slsa.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@chtiangg
Copy link
Contributor Author

chtiangg commented Jun 7, 2023

Sorry, included a github action workflow script by mistake, and removing it now.

@chtiangg
Copy link
Contributor Author

chtiangg commented Jun 7, 2023

Well, had a pretty bad experience with git rebase, but made it work.

@chtiangg chtiangg changed the title Add more charities for build model terminologies (issue #714) Adds more charities for build model terminologies (issue #714) Jun 7, 2023
Copy link
Member

@joshuagl joshuagl 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 working on these clarifications, these changes are a solid improvement.
I have a few minor requests and a couple of questions, LMK what you think.

docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/build-model.svg Outdated Show resolved Hide resolved
@lehors lehors changed the title Adds more charities for build model terminologies (issue #714) Adds clarity to build model terminology (issue #714) Jun 9, 2023
Copy link
Contributor Author

@chtiangg chtiangg left a comment

Choose a reason for hiding this comment

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

I will hold off on making any changes, before we resolve the comment thread for the "steps".

docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/build-model.svg Outdated Show resolved Hide resolved
Copy link
Contributor Author

@chtiangg chtiangg 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 the review! I will update the description and remove the reverted changes.

docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
Copy link
Member

@MarkLodato MarkLodato left a comment

Choose a reason for hiding this comment

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

LGTM. I just pushed a few minor fixes, so another review by @joshuagl would be appreciated.

@MarkLodato MarkLodato requested a review from joshuagl June 23, 2023 20:21
docs/spec/v1.0/terminology.md Outdated Show resolved Hide resolved
Co-authored-by: Joshua Lock <[email protected]>
Signed-off-by: Mark Lodato <[email protected]>
@MarkLodato MarkLodato merged commit 5e7d274 into slsa-framework:main Jun 26, 2023
@chtiangg chtiangg deleted the build-model branch July 3, 2023 10:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants