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

Add PROCESS.md documenting the new SDLC #1348

Closed
wants to merge 3 commits into from

Conversation

jdesrosiers
Copy link
Member

@jdesrosiers jdesrosiers commented Nov 16, 2022

Resolves: #1334

This is the formal documentation of the process we've discussed in https://github.com/orgs/json-schema-org/discussions/234. That discussion was focused on getting agreement on the general direction, while this makes some concrete decisions. Please do challenge any of the concrete decisions I've made here, but keep in mind that this is just a starting point, so it might be more productive address issues after merge as separate PRs/Issues so we don't end up with multiple threads of discussion in this PR.

Several more concrete decisions still need to be made that should be addressed in the spec (for example: dialect and vocabulary URIs). Those decisions are out of scope for this document.

I made some minor changes from what we had discussed to how STAGE-2 is defined, so pay particular attention to that section when reviewing.

Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

We mention "JSON Schema Org core contributors" in several places. Do we have/need a process to determine on- and offboarding of people into this role?

Also, a lot of my questions may be candidates for issues. Feel free to indicate which ones you'd like to defer.

PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Outdated
Comment on lines 27 to 29
deprecated. Flags are used to show the status a feature is in. If a feature
doesn't have a flag, it's considered stable. If it has a year flag (such as
`2024`) that's the year the feature reached stability. The other flags indicate
Copy link
Member

Choose a reason for hiding this comment

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

If we're considering flagless features as stable, is there another indicator as to when the feature reached stability? Or does "doesn't have a flag" only pertain to the status flag? Do we expect all stable features to have a year flag? If one doesn't is that just an admin error that needs to be fixed?

Maybe it's easier to require that features have flags so that if they don't it's something to be fixed. Having an allowance for not having a flag seems to raise a lot of questions.

Copy link
Member Author

Choose a reason for hiding this comment

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

A part of the spec can only be unflagged if it's a stable feature that was introduced in the original stable release.

My thinking was that it would be too noisy to flag everything in the spec. It would be hard to see 2024 flags in a sea of 2023 flags. The first stable release wouldn't have any 2023 flags because it's unambiguous when those features were introduced since 2023 is the first release. Stable flags would start to be added for 2024. That means that the number of flags in the spec should be fairly small and therefore stand out to readers.

I'll revise to clarify the purpose of unflagged features, but let's continue the discussion and get some more opinions if you still dislike the idea.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we keep the "unflagged implies stable" idea, and instead of flagging the year for future features, we have a newly-stable flag that identifies features that are new for the year's release. Then the next year, those newly-stable flags are cleared from those features so that the new newly-stable features stand out.

Copy link
Member

Choose a reason for hiding this comment

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

Your changes look fine.

Copy link
Member Author

Choose a reason for hiding this comment

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

Let's discuss it at the next OCWM

PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Show resolved Hide resolved
PROCESS.md Show resolved Hide resolved
PROCESS.md Outdated Show resolved Hide resolved
PROCESS.md Show resolved Hide resolved
PROCESS.md Outdated Show resolved Hide resolved
Co-authored-by: Greg Dennis <[email protected]>
Co-authored-by: Julian Berman <[email protected]>
Copy link
Member

@gregsdennis gregsdennis left a comment

Choose a reason for hiding this comment

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

The changes that have been made are good. I don't think the remaining items warrant holding this up.

Copy link
Member

@Relequestual Relequestual left a comment

Choose a reason for hiding this comment

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

Left some comments and questions.

Some parts feel under specified, which is fine for a draft, but I feel like there are some key issues to iron out.

Additionally, I feel like we should be stating...
"This policy document uses RFC2119 defined key words to indicate requirement levels" =]

Bulk of this looks like a great first start though. Thanks for all your work on this!

PROCESS.md Outdated
Comment on lines 31 to 38
deprecated. Flags are used to show the status a feature is in. If it has a year
flag (such as `2024`) that's the year the feature reached stability. The other
flags indicate a feature that is not yet stable or is deprecated.

If a feature doesn't have a flag, it's considered stable. A feature can only be
unflagged if it was stable in the initial stable release in 2023. Any functional
change to the spec after the initial stable release MUST have a flag indicating
it's status.
Copy link
Member

Choose a reason for hiding this comment

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

If it has a year flag (such as 2024) that's the year the feature reached stability.

If a feature doesn't have a flag, it's considered stable. A feature can only be
unflagged if it was stable in the initial stable release in 2023.

Under what situation would a feature have no flag?

Copy link
Member Author

Choose a reason for hiding this comment

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

You quoted the answer immediately before asking the question, so I'm not sure how to answer.

A feature can only be unflagged if it was stable in the initial stable release in 2023.

I can expand on that if it's not clear enough, but I'm not sure what's not clear. Does this discussion with @gregsdennis help? #1348 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

I think I mistook "unflagged" for "deflagged".
In which case, I'm not sure I find this the best approach.

Copy link
Member Author

@jdesrosiers jdesrosiers Nov 28, 2022

Choose a reason for hiding this comment

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

I see. I'll try to reword to avoid that misunderstanding. Removing or changing flags isn't disallowed, but compatibility rules apply. So, you couldn't remove a stable flag (such as 2024) because it would be a backward-incompatible change, but you could change a STAGE-2 to a 2024 because STAGE-2 is allowed backward-incompatible changes. Similarly, you couldn't remove a STAGE-2 flag without replacing it with a stable flag because that would break backward compatibility.

Copy link
Member Author

Choose a reason for hiding this comment

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

Here's an example of the kind of thing I have in mind from the node.js docs https://nodejs.org/dist/latest-v19.x/docs/api/fs.html. They have stable, unstable, and deprecated features and flag them inline just like we're talking about here.

PROCESS.md Outdated
Comment on lines 40 to 45
### Champions
Any feature that is evolving SHOULD have a champion. The champion is responsible
for shepherding the feature through the process to into stable status. The
champion doesn't need to do all the work, they're just responsible for making
sure the work gets done. The champion role MAY be shared by more than one
person. An evolving feature without a champion SHOULD be removed from the spec.
Copy link
Member

Choose a reason for hiding this comment

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

I would like to see that first requirement as a MUST.
I guess it was framed this way to allow for a change of champion should an existing champion want to step down or vanish?
I'd much rather be stricter here and spell out even a workflow for changing champions and putting a feature "on hold" or some such until a champion is found/appointed.

Copy link
Member Author

Choose a reason for hiding this comment

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

I fully expect champions to disappear on occasion. In that case, a new champion would be named or the feature would be abandoned and removed. The "SHOULD" is to give us some leeway if for some reason we can't or don't want to assign a champion right away, but also don't want to remove the feature. It's not a likely scenario. I'm fine with calling it a MUST if we want to.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather call it a MUST. As for remaining in the spec, I think that would need to be a case by case basis.

Copy link
Member Author

Choose a reason for hiding this comment

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

If having a champion is a MUST, that means if a feature loses its champion for some reason, our only choices are to assign a new champion or remove it from the spec. If we don't remove it from the spec, then we are in violation of the MUST. If you want spec removal to be on a case-by-case basis rather than a MUST remove, we need to go with "SHOULD" to give us that option.

PROCESS.md Outdated
Comment on lines 103 to 104
[] There is general consensus that the feature has been proven to be a good
addition to JSON Schema and is unlikely to change.
Copy link
Member

Choose a reason for hiding this comment

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

...general consensus...

Determined by whom?

Copy link
Member Author

Choose a reason for hiding this comment

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

Whoever the JSON Schema Org active contributors happen to be at the time. Maybe at some point that will be a specific and official group of people like a TSC, but for now it's squishy concept. For now I'll make the text more specific. We can update the wording when/if we have an official group we can name.

PROCESS.md Outdated
**Promotion to STAGE-2 Checklist:**
[] There is general consensus that the feature has been proven to be a good
addition to JSON Schema and is unlikely to change.
[] We see the feature being used successfully in the wild and not generating a
Copy link
Member

Choose a reason for hiding this comment

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

We?

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

OK, then that needs to be defined.

PROCESS.md Outdated
`STAGE-2` features can only be promoted to stable as part of a release.

**Promotion to Stable Checklist:**
[] The vast majority of actively maintained implementations support the feature
Copy link
Member

Choose a reason for hiding this comment

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

I feel like we need to define...
"vast majority"
"activitly maintained"
"support the feature"

As an aside, I'm concerned thinking about "what if implementers don't implement?" But I think we should investigate further sponsoring implementation work at some point, which could aid this.

Copy link
Member Author

Choose a reason for hiding this comment

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

"vast majority"

see #1348 (comment)

"actively maintained"

Yep. The more I think about it, the more I agree that this needs more definition. For example, an implementation may be active for bug fixes, but don't intent to add new features. Should that count? This probably needs discussion and probably doesn't need to be decided now as it will be a while before we have to make this determination.

"support the feature"

The meaning of those words seem pretty clear to me. We could explicitly say it has to pass all the tests. Is there more nuance that I'm missing?

I think we should investigate further sponsoring implementation work at some point

Sure. I think that speaks to increasing the pool of "actively maintained" implementations.

Comment on lines +162 to +165
Meta-schemas associated with the specification(s) maintained under this process
are subject to the same compatibility rules as the specification. They can be
updated at any time, but changes MUST be backward compatible and allow for
forward compatibility with future schemas.
Copy link
Member

Choose a reason for hiding this comment

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

I have some questions/ comments here.

Are we going to maintain meta-schemas attached to individual release snapshots?
Are we going to have a "current" meta-schema which contains all keywords, even stage-0?
Are we going to have a "current" meta-schema which per stage? For example, all stage-1 and above or all stage-2 and above?

If we have a "current" meta-schema, which includes all of the stages, how can we update meta-schemas yet maintain backwards compatiblity?
This becomes a very specific problem if we disallow unknown keywords by default.

What if we wanted to update the meta-schemas to use a new keyword? That wouldn't be backwards compatible.

I think backwards compatible meta-schemas only makes sense for a given release.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just like the spec, there is one set of mutable meta-schemas that describe the current state of the specification. Because of backward and forward compatibility constraints, no matter what release of the spec the schema is written for, the latest meta-schemas is all you need.

Are we going to maintain meta-schemas attached to individual release snapshots?

We could have snapshots for reference like with the spec, but there wouldn't be reason to actually use those snapshots for anything other than historical reference.

Are we going to have a "current" meta-schema which contains all keywords, even stage-0?
Are we going to have a "current" meta-schema which per stage? For example, all stage-1 and above or all stage-2 and above?

The meta-schemas should include anything that's in the spec. That includes STAGE-1, STAGE-2, and deprecated.

If we have a "current" meta-schema, which includes all of the stages, how can we update meta-schemas yet maintain backwards compatiblity?

Meta-schema changes should be backward compatible because spec changes are backward compatible.

This becomes a very specific problem if we disallow unknown keywords by default.

What if we wanted to update the meta-schemas to use a new keyword? That wouldn't be backwards compatible.

I was thinking that the standard dialect meta-schema would allow additional keywords. If a keyword appears in the meta-schema but is not supported by the implementation, the implementation should raise an error. It's not something the meta-schema can know, so I consider it a runtime constraint the implementation needs to deal with, not the meta-schema.

However, meta-schemas could disallow additional keywords and it shouldn't hurt anything. If an implementation is using an older version of the meta-schema that doesn't have a newer keyword used by the schema, then the implementation also wouldn't have support for the keyword. The result is always that you get an error that the implementation doesn't know the keyword.

Either way, there shouldn't be a backward or forward compatibility issue and whether we choose to allow additional keywords in the meta-schema is not relevant to process, so we can discuss that when we get to writing the meta-schemas.

Copy link
Member Author

Choose a reason for hiding this comment

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

I expect there to be some discussion about meta-schema lifecycle that may end up as a bigger conversation. This was the kind of thing I thought might be best to get into after the initial merge. Like I said, I don't expect merging this document to mean it's "done". I expect it to be a first draft that we will iterate on.

Copy link
Member

Choose a reason for hiding this comment

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

There are a number of assertions in your response which I don't agree with, but I don't think here's the right place to capture this discussion.

I expect there to be some discussion about meta-schema lifecycle that may end up as a bigger conversation. This was the kind of thing I thought might be best to get into after the initial merge. Like I said, I don't expect merging this document to mean it's "done". I expect it to be a first draft that we will iterate on.

I think we need to discuss how we can break up the SDLC into more discreet chunks for further discussion.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's exactly why I proposed merging this as a work-in-progress and opening issues to discuss the parts that need more discussion. Holding this up until we decide every detail makes it more difficult to have those discussions.

@gregsdennis
Copy link
Member

@Relequestual have a read through the comments that I made and the responses given. Maybe that will answer some of yours.

@Relequestual
Copy link
Member

@jdesrosiers Can this be closed in favour of #1500 ?

@jdesrosiers
Copy link
Member Author

Yes, it can

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.

Formally document the new SDLC
4 participants