ENH: 01 #1
ENH: 01 #1
Conversation
There was a problem hiding this comment.
Great start, thanks @SantiagoTorres!
I have two major questions:
-
I think we should emphasize and discuss backwards-incompatibility a bit more. It is very important than an ITE does not break backwards-incompatibility unless absolutely necessary, and there is no other better way.
-
Should we have timelines for how long an ITE is allowed to spend at most in each state? This would guarantee some sort of an SLA for the whole process.
@ofek What do you think?
Thanks,
Trishank
ITE/1/README.adoc
Outdated
|
|
||
| Vetting an idea publicly before going as far as writing a ITE is meant | ||
| to save the potential sponsor time. Many ideas have been brought | ||
| forward for changing Jenkins that have been rejected for various |
There was a problem hiding this comment.
@SantiagoTorres: Before you change this in your fork, please take a look at mine and cherry pick instead (see lukpueh@91d48a6)
ITE/1/README.adoc
Outdated
| [[submission]] | ||
| ==== Creating a ITE Submission | ||
|
|
||
| Following a discussion on [email protected], |
ITE/1/README.adoc
Outdated
| Even after a ITE reaches "Final" status, it may need to be updated. | ||
|
|
||
| In general, Standards track ITEs are not modified after they have | ||
| reached the Final state. Once a Standards ITE has been completed, Jenkins developer |
ITE/1/README.adoc
Outdated
| being addressed. | ||
| . **Specification** - The technical specification should describe the | ||
| syntax and semantics of any new feature. The specification should be | ||
| sufficiently detailed to allow new or existing Jenkins developers to |
ITE/1/README.adoc
Outdated
| sufficiently detailed to allow new or existing Jenkins developers to | ||
| reasonably understand the scope/impact of an implementation. | ||
| . **Motivation** - A clear description of the motivation is critical for any ITE | ||
| that wants to change Jenkins or the Jenkins project. |
ITE/1/README.adoc
Outdated
| sections will not be approved as ITE Drafts. | ||
|
|
||
| The final implementation must include test code and documentation | ||
| appropriate for either the Jenkins user or developer documentation. |
ITE/1/README.adoc
Outdated
| A **Resolution** section will be added to ITEs when their status is set to | ||
| Accepted, Rejected or Withdrawn. | ||
| It will include a link to the relevant post in the | ||
| [email protected] mailing list archives. |
There was a problem hiding this comment.
wow, I'm not sure how I missed these. Apologies
This document is based on the jenkins enhancement proposal (JEP) and did not replace all occurrences of jenkins that should be in-toto. This commit fixes this addressing @trishankatdatadog's review comments: in-toto#1 (review)
- Remove stray words. - Add missing words. - Fix some commas, periods, apostrophes.
There was a problem hiding this comment.
Great work! I fixed some minor issues in my fork (feel free to cherry-pick from lukpueh/ITE@jep-xxxx-lp) and made comments below.
ITE/1/README.adoc
Outdated
|
|
||
| ITE stands for in-toto Enhancement. An ITE is a design document that describes | ||
| a new feature or aspect of the in-toto framework, the in-toto supply chain, or | ||
| the process within in-toto project. An ITE provides a concise technical |
There was a problem hiding this comment.
"the process within in-toto project"
What does does mean?
There was a problem hiding this comment.
I.e., passing ITE's, or other governance issues that we may want to define.
ITE/1/README.adoc
Outdated
| changing their status). See <<editor-responsibilities, ITE Editor Responsibilities & Workflow>> for | ||
| details. | ||
|
|
||
| ITE editorship is by invitation of the current editors. All of the ITE workflow |
There was a problem hiding this comment.
editorship is [given, assigned, received, ... ?] by invitation
ITE/1/README.adoc
Outdated
| ==== Reviewer | ||
|
|
||
| The ITE Reviewer is the contributor who will make the final decision whether to | ||
| accept a ITE. In all cases where this document refers to the `Reviewer`, it |
There was a problem hiding this comment.
Do we care for uniform emphasis and capitalization for terminology, e.g.
Editor vs. "Sponsor" vs. Reviewer vs. contributor
There was a problem hiding this comment.
Above comment applies to other terminology as well, e.g. ITE type, status, links to headings, ...
There was a problem hiding this comment.
I think it'd be good to have it be uniform
ITE/1/README.adoc
Outdated
| or offer feedback about that ITE. One contributor might do only one or any | ||
| combination of these of these things during any part of the life of a ITE. | ||
| While we invite contributions by companies or other organizations, contributors | ||
| listed in a ITE need to be specific people. |
There was a problem hiding this comment.
Do I understand correctly that all roles are also contributors? Should we mention this?
There was a problem hiding this comment.
Yes, added text to specify this
ITE/1/README.adoc
Outdated
| forums, and attempts to build community consensus around the idea. The ITE | ||
| Sponsor should first attempt to ascertain whether the idea is ITE-able. | ||
| Posting to the [email protected] mailing list or opening an issue in | ||
| the specification repository footnoteref:[repo] is |
There was a problem hiding this comment.
I think the footnote should point to a link to https://github.com/in-toto/docs
There was a problem hiding this comment.
Sounds fair, will update.
|
|
||
| Active:: [[active]] | ||
| Some Informational and Process ITEs may have a status of "Active" instead of "Final" | ||
| These ITEs are ongoing and never meant to be completed per se. E.g. ITE 1 (this ITE). |
There was a problem hiding this comment.
[were?] never meant to be completed?
Also, not sure if "per se" is the right expression here. Did you mean "a priori"?
There was a problem hiding this comment.
Hmm? No, it means that an ITE can be ammended and such.
| ITEs may also have a **Superseded-By** row indicating that a ITE has been | ||
| rendered obsolete by a later document; the value is the number of the ITE that | ||
| replaces the current document. The newer ITE must have a **Replaces** row | ||
| containing the number of the ITE that it rendered obsolete. |
There was a problem hiding this comment.
Should we add an additional header rows subsection:
Replaces:: [[header-replaces]]
?
There was a problem hiding this comment.
Well, only if it replaces anything no? Or would you rather have "none" or so in that row if it replaces anything?
There was a problem hiding this comment.
I meant add in order to describe, like you are describing all the other available "Additional header rows" here, e.g something like
Replaces:: [[header-replaces]]
An ITE that replaces another ITE must have a **Replaces** row containing the number of the ITE that it rendered obsolete.
(Not add because this particular ITE replaces another ITE) :)
ITE/1/README.adoc
Outdated
| send comments and changes directly to the ITE sponsor. For more mature, or | ||
| finished ITEs consider submitting corrections to the ITE repository | ||
| footnoteref:[repo] or the in-toto issue tracker | ||
| footnoteref:[issues, https://github.com/in-toto/ITE/issues]. If the ITE sponsor is an |
There was a problem hiding this comment.
footnoteref:[issues, https://github.com/in-toto/ITE/issues]
should probably be
footnoteref:[issues, https://github.com/in-toto/in-toto/issues]
There was a problem hiding this comment.
Hmm, I'm not sure as to why this is? care to elaborate please?
|
|
||
| == Motivation | ||
|
|
||
| in-toto has classically been driven by "you-had-to-be-there" development. With |
ITE/1/README.adoc
Outdated
| specific changes largely being driven by smaller independent groups of | ||
| developers (sometimes just one). Sometimes, decisions were made and not | ||
| properly documented, which resulted in additional overhead and mind-reading | ||
| efforts to dig long-established rationales. |
ITE/1/README.adoc
Outdated
| == Abstract | ||
|
|
||
| An in-toto Enhancement (ITE) is a design document that describes a new feature | ||
| or aspect of the in-toto framework, a setup of its supply chain or an, |
There was a problem hiding this comment.
Please consider taking a look at my proposed fixes (lukpueh/ITE@jep-xxxx...lukpueh:jep-xxxx-lp).
Above issue, for instance, is already fixed.
There was a problem hiding this comment.
Oh, I wasn't aware of that. I only saw your comments directly on this page.
Is there a direct link from this page to all proposed fixes, such as yours? (otherwise, how can I see all proposed fixes?)
There was a problem hiding this comment.
See my review summary above, i.e. #1 (review)
ITE/1/README.adoc
Outdated
| described below. An ITE may also have any number of <<Contributor, | ||
| Contributors>> who help write, implement, discuss, or offer feedback about | ||
| the ITE. One contributor might do only one or any combination of these things | ||
| during any part of the life of a ITE. |
There was a problem hiding this comment.
Earlier in this paragraph, we used "An ITE" ("What is an ITE?"). You should change "a ITE" to "an ITE" throughout this document for consistency. I assume this can be done easily using search & replace functionality?
ITE/1/README.adoc
Outdated
| . An **Informational** ITE describes an in-totodesign issue, or | ||
| provides general guidelines or information to the in-toto community, | ||
| but does not propose a new feature. Informational ITEs do not | ||
| necessarily represent a in-toto community consensus or |
There was a problem hiding this comment.
change "a in-toto" to "an in-toto"?
Change this throughout this document for consistency.
ITE/1/README.adoc
Outdated
| for the in-toto specification. It may also describe an interoperability or | ||
| backwards-compatibility standard which will be supported for the feature in | ||
| current versions of in-toto, moving forward. | ||
| . An **Informational** ITE describes an in-totodesign issue, or |
ITE/1/README.adoc
Outdated
| but does not propose a new feature. Informational ITEs do not | ||
| necessarily represent a in-toto community consensus or | ||
| recommendation, so users and implementers are free to ignore | ||
| Informational ITEs or follow their advice. An example of an informational ITE |
There was a problem hiding this comment.
Use "Informational ITE" or "informational ITE" throughout this document?
ITE/1/README.adoc
Outdated
| For example, one sponsor might write large portions of one ITE, while another | ||
| sponsor might leave the writing to other contributors. | ||
|
|
||
| Anyone may be Sponsor for a ITE, though it should be someone familiar enough |
ITE/1/README.adoc
Outdated
| Before delving into the details of the ITE workflow, let's take a high-level | ||
| look at how ITE might go. | ||
|
|
||
| . **<<start, Initial Discussion>>** - Andrea has an idea for new feature and emails it [email protected]. |
ITE/1/README.adoc
Outdated
|
|
||
| . **<<review, Review>>** - Kelly reviews the ITE and any related discussions | ||
| and implementation. Kelly agrees with Andrea that consensus has been reached | ||
| regarding the ITE and that the implementation is far enough along to enusure |
ITE/1/README.adoc
Outdated
|
|
||
| By marking an ITE as "Accepted" the Reviewer indicates they believe that the | ||
| ITE has clear scope, design completeness, community consensus, and (if | ||
| applicable) in-progress implementation. Without all of these a ITE cannot be |
There was a problem hiding this comment.
I think the dash character is not necessary for "in-progress"
ITE/1/README.adoc
Outdated
| original sponsor. A good reason to transfer sponsorship is because the | ||
| original sponsor no longer has the time or interest in updating it or | ||
| following through with the ITE process, or has fallen off the face of | ||
| the 'net (i.e. is unreachable or not responding to email). A bad |
There was a problem hiding this comment.
incomplete single quote (before "net")?
ITE/1/README.adoc
Outdated
| === Benefits to future developers | ||
|
|
||
| By providing clear, understandable, and bite-sized design documents which would | ||
| explain various subsections of in-toto. ITEs also make it clearer how an |
There was a problem hiding this comment.
Change dot (.) to comma (,) before "ITEs"?
There was a problem hiding this comment.
I'm not sure about this one. Care to elaborate about the rationale ? :)
There was a problem hiding this comment.
"By providing clear, understandable, and bite-sized design documents which would
explain various subsections of in-toto." sounds like an incomplete sentence. Something it's missing.
So I assumed, your intention was to link the above with the next sentence.
There was a problem hiding this comment.
ah, let me clean this up
|
|
||
| There are no new infrastructure requirements related to this proposal. | ||
| This ITE leverages existing infrastructure. | ||
|
|
There was a problem hiding this comment.
Do you need to include a "== Testing section", and say this ITE has no testing needs? (based on the earlier definitions)
There was a problem hiding this comment.
I think that's implied that since this doesn't include code testing isn't included. Would you want me to rephrase the definition or add the header to reflect this?
There was a problem hiding this comment.
Earlier in this document, under "ITE Guidelines", "Required Sections", you say that for Testing:
"If the ITE has no testing needs, the section must say that."
Similarly, for Backwards Compatibility, you say that:
"If there are no backwards compatibility concerns, the section must say that."
And for Security:
"If the ITE has no impact on security, the section must say that."
So, I see that for this ITE, you DID include sections for Security and Backwards Compatibility, which say that there is no impact, but you did not do that for Testing.
There was a problem hiding this comment.
right. Sorry I'll fix this
ITE/1/README.adoc
Outdated
| * Coordinating contributors' work | ||
| * Ensuring the ITE meets the style, format, and quality guidelines | ||
| * Maintaining the ITE after it is finalized | ||
| * Setting and communicating the schedule as needed |
There was a problem hiding this comment.
Not clear what this means, i.e. what "schedule" does it refer to.
There was a problem hiding this comment.
As to when reviews are due. I tried to rephrase to make this clearer
| NOTE: When choosing to go counter to SHOULD or SHOULD NOT guidance, | ||
| the reasons behind that choice SHOULD be documented. | ||
|
|
||
| === ITE Workflow |
There was a problem hiding this comment.
As part of an ITE workflow section, it seems that for an ITE to be approved, it must contain both design AND implementation for a new feature. If one just wants to propose a new feature by describing the design from a specification p.o.v, but doesn't provide an implementation, that cannot be done through the ITE process.
This is not a major concern, I'm just wondering if this is what we want. In fact, reading the rest of this document, it seems that not all ITEs must have an implementation, but the ITE workflow section suggests they must have one.
ITE/1/README.adoc
Outdated
|
|
||
| Deferred:: [[deferred]] | ||
| A ITE can also be assigned a status of "Deferred". The ITE sponsor or an | ||
| editor can assign the ITE this status when no progress is being made |
There was a problem hiding this comment.
Do you want to define a timeframe for when this can happen? 6 months? 1 year?
There was a problem hiding this comment.
I vote for 6 months, but I'd like to have the opinion of other stakeholders on this decision :)
|
Great job! |
|
@reza-curtmola @JustinCappos @lukpueh @trishankatdatadog Thanks for your review! I updated the document and hopefully we are close to having something final? There's mostly two issues that I need discussion:
Please do review the template too :) |
|
Regarding the two issues:
|
|
2. For backwards compatibility, I propose paraphrasing from [TAP 1](https://github.com/theupdateframework/taps/blob/master/tap1.md): "All ITEs that introduce backwards incompatibilities must include a section describing these incompatibilities, justifying their necessity, and their severity. The ITE must explain how the Sponsor proposes to deal with these incompatibilities. ITEs without a sufficient backwards compatibility treatise will be rejected outright."
How does this differ from the existing description of the backwards
compatibility section?
Thanks,
-Santiago.
|
- Add optional replaces header - Clarify sentence in motivation for this ITE - Add the missing testing section
This adds an expected timeline section to specify what's the expected time to spend on each section of the ITE admission process.
|
LGTM, man, great job! 👍 |
|
@JustinCappos @reza-curtmola @lukpueh any objections? :) |
|
Looks good to me. Should I merge? |
|
Sounds good to me 👍 please do |
|
Sorry for the late review. You did a great job, Santiago, and everything looks fine. I found a few really minor typos and wording issues and proposed fixes in #2. |
Update readme
@in-toto/ite-editors
Here's the draft of the first ITE (In-toto Enhancement). It's heavily based off of the JEP format. Please everyone comment on it!
@lukpueh @reza-curtmola @trishankatdatadog @ofek