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

Formalize some feature guidelines #334

Merged
merged 1 commit into from
Sep 26, 2018
Merged

Formalize some feature guidelines #334

merged 1 commit into from
Sep 26, 2018

Conversation

KnVerey
Copy link
Contributor

@KnVerey KnVerey commented Sep 19, 2018

This is something I've been meaning to propose for a while, both to help contributors and to sync up as maintainers.

  • Is this readme section the right place for this content?
  • Anything I missed?
  • Anything you disagree with?

cc @Shopify/cloudx

Copy link
Contributor

@kirs kirs left a comment

Choose a reason for hiding this comment

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

It's great to have this as a statement. Thanks for adding!


- This project's mission is to make it easy to ship changes to a Kubernetes namespace and understand the result. Features that introduce new classes of responsibility to the tool are not usually accepted.
- Deploys can be a very tempting place to cram features. Imagine a proposed feature actually fits better elsewhere—where might that be? (Examples: validator in CI, custom controller, initializer, pre-processing step in the CD pipeline, or even Kubernetes core)
Copy link
Contributor

Choose a reason for hiding this comment

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

Well put ❤️

- The basic ERB renderer included with the tool is intended as a convenience feature for a better out-of-the box experience. Providing complex rendering capabilities is out of scope of this project's mission, and enhancements in this area may be rejected.
- The deploy command does not officially support non-namespaced resource types.
- This project strives to be composable with other tools in the ecosystem, such as renderers and validators. The deploy command must work with any Kubernetes templates provided to it, no matter how they were generated.
- This project is open-source. Features tied to any specific organization (including Shopify) will be rejected.
Copy link
Contributor

Choose a reason for hiding this comment

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

Features tied to any specific organization (including Shopify) will be rejected.

While this is a good north start to have, we already do have some Shopify specific features, like ejson handling, Memcache and Redis resource types. What do we do about it? Do we want to mention that the rule only applies to new features?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point, although ejson is also open-source so I think it's ok, and we're ripping out the proprietary CR code "Soon" (by reviving #188, more or less).

Copy link
Contributor

@dturn dturn left a comment

Choose a reason for hiding this comment

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

Small wording suggestion, 👍 on adding this information.


- This project's mission is to make it easy to ship changes to a Kubernetes namespace and understand the result. Features that introduce new classes of responsibility to the tool are not usually accepted.
- Deploys can be a very tempting place to cram features. Imagine a proposed feature actually fits better elsewhere—where might that be? (Examples: validator in CI, custom controller, initializer, pre-processing step in the CD pipeline, or even Kubernetes core)
Copy link
Contributor

Choose a reason for hiding this comment

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

Imagine a proposed feature actually fits better elsewhere—where might that be? -> Does the proposed feature actually fits better elsewhere?

@KnVerey KnVerey merged commit c64af56 into master Sep 26, 2018
@KnVerey KnVerey deleted the guidelines branch September 26, 2018 21:13
@fw42 fw42 temporarily deployed to rubygems October 16, 2018 10:39 Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants