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

November 22, 2021 Community Meeting #42

Closed
SaschaSchwarze0 opened this issue Nov 22, 2021 · 6 comments
Closed

November 22, 2021 Community Meeting #42

SaschaSchwarze0 opened this issue Nov 22, 2021 · 6 comments

Comments

@SaschaSchwarze0
Copy link
Member

  • Please add a topic in this thread and add a link to the Github issue associated with the topic.
  • Please make sure you give folks enough time to review/discuss the topic offline on Github before coming into the meeting
  • (optional) Paste the image of an animal
@SaschaSchwarze0
Copy link
Member Author

Optional fields in our custom resources, see CRD usage of pointers vs non-pointers #945.

Based on Kubernetes API conventions:

we ask that pointers always be used with optional fields that do not have a built-in nil value

I can understand this, but it imo has disadvantages. Let's take a string as example. In many cases where we have optional strings, we do not need to distinguish between nil and "". An example is the Build's spec.source.revision. We allow users to specify it or to leave it out. But, an empty string is invalid. We would need additional validation for those use cases.

How do we want to define our own API convention? Based on what Kubernetes community suggests, or slightly different = use pointers for optional fields where you need to distinguish nil and the built-in nil value, for example "" for strings.

cc @shahulsonhal

🐩

@shahulsonhal
Copy link
Member

Replace the existing linters (sanity-check) with all in one golangci-lint

@SaschaSchwarze0
Copy link
Member Author

Looking for an approval of SHIP-0025: Add ship to support array strategy parameters and secret and configmap key references as value #34. Gabe and Matthias approved. Need Adam or Shoubhik to give blessing.

🐴

@HeavyWombat
Copy link
Contributor

HeavyWombat commented Nov 22, 2021

Any scans for licenses or patent issues for modules?
🐈

@SaschaSchwarze0
Copy link
Member Author

SaschaSchwarze0 commented Nov 22, 2021

Pointers topic:

  • We make optional fields generally pointers.
  • Shoubhik: For fields where nil and "" do not need to be distinguished, we interpret "" as nil (like for the revision)
  • Matthias: prefer an empty string to be interpreted as an empty string
  • Shoubhik: what would be the expectation of the user specifying an empty string ?
  • For build.spec.dockerfile, we have no empty string check atm
  • Decision: Use pointers, do not interpret "", check code locations if validation is missing

golintci-lint:

Shoubhik will look at the ship

Scans for licenses/patent issues:

@sbose78
Copy link
Member

sbose78 commented Nov 22, 2021

Follow-up from today's discussion on image-spec shipwright-io/build#947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants