Skip to content

Add essential policies to DEVELOPMENT.md #3599

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

Merged
merged 2 commits into from
Mar 5, 2024
Merged

Add essential policies to DEVELOPMENT.md #3599

merged 2 commits into from
Mar 5, 2024

Conversation

handrews
Copy link
Member

In yet another attempt to prevent well-meaning people from opening PRs that we cannot accept. It turns out that if you make a PR template, it puts the template info below the commit message so people often don't see it.

@handrews handrews requested review from a team as code owners February 20, 2024 22:54
In yet another attempt to prevent people from opening PRs that
we cannot accept.
@lornajane
Copy link
Contributor

We could also name this file CONTRIBUTING to help users find it in a standard location (see https://docs.github.com/en/communities/setting-up-your-project-for-healthy-contributions/setting-guidelines-for-repository-contributors) - I assume the convention is newer than the original structure of this project.

Copy link
Contributor

@lornajane lornajane 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 a commitment to update the list of open branches, but I think it's worth it since we have to type it into half the pull requests we get anyway. I made a small suggestion, happy to review again at any time.

DEVELOPMENT.md Outdated

If in doubt about a policy, please [ask on our Slack](https://communityinviter.com/apps/open-api/openapi) before opening a PR.

_Note that the later sections of this document have not been updated recently. Until that has happened, if this section and later sections disagree, this section is to be considered the current policy._
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd discourage adding time-sensitive content into a file we will probably continue to fail to update. Directing people to slack is the right thing in almost every case of confusion!

Copy link
Member Author

Choose a reason for hiding this comment

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

@lornajane yeah... I'm not sure what to do here. This file is huge, and a lot of it needs rewriting. I'd almost rather delete everything that's not current. Better to have undefined processes that documentation that is, at this point, outright wrong.

@handrews
Copy link
Member Author

@lornajane I'd be quite happy to move it to CONTRIBUTING. I thought we had one that was used for something else, but we have CONTRIBUTORS. Which is also not used in a normal way as it's the TSC list, not the contributor list.

I'm also hesitant to add time-sensitive information, but like you said, we have to type it in so often anyway, at least this way we could point to an "official" document. I've noticed not everyone will go to Slack, much less ask a question there.


#### Changing the schemas

Schemas are only changed _after_ the specification is changed. Changes are made on the `main` branch, and should be made to the YAML version _only_. The JSON version will be generated automatically.
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't sound right because there are a number of schema changes in the v3.1.1-dev branch (mostly fixes to restore conformance with 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.

Also - could there/should there be an automated script that updates the json file immediately after a merged change to the yaml file?

Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't sound right because there are a number of schema changes in the v3.1.1-dev branch (mostly fixes to restore conformance with the spec).

I think those are ones that got merged by accident because someone didn't see that the branch was wrong. Although for 3.2 you are probably right, as it will get a new schema (but patch releases do not). Honestly, I'm not sure, I'm kind of trying to provoke some sort of clear policy.

Also - could there/should there be an automated script that updates the json file immediately after a merged change to the yaml file?

That would be great, but would require someone to do it. We could use forward-porting scripts that are more per-change oriented than the ones in the scripts directory and automation to forward-port individual PRs as well.

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 those are ones that got merged by accident because someone didn't see that the branch was wrong.

I believe they were intentional.. there was even a new interim release made of the revised schema (about a year ago now) - "$id": "https://spec.openapis.org/oas/3.1/schema/2022-10-07". There have been enough commits since then that we could have another interim release now.

@lornajane
Copy link
Contributor

I am not sure I've ever read this whole page before, we can definitely lose some of it! Your additions are a good enhancement so let's start there. I still think we should avoid the time-sensitive disclaimer, but without it, I'm happy and the other changes can follow.

@lornajane lornajane self-requested a review February 29, 2024 17:32
Copy link
Contributor

@lornajane lornajane left a comment

Choose a reason for hiding this comment

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

This is very good as it stands, thanks for adding it!

@AxelNennker
Copy link
Contributor

The two tables "Current branches and documents open to change" and https://github.com/OAI/OpenAPI-Specification/blob/4a11c66836f7e3a0b9246a98849428bbc55ea8fd/DEVELOPMENT.md#tracking-process seem to be at odds.

At any given time, there would be at most 4 work branches. The branches would exist if work has started on them. Assuming a current version of 3.0.0:

    main - Current stable version. No PRs would be accepted directly to modify the specification. PRs against supporting files can be accepted.

    v3.0.1-dev - The next PATCH version of the specification. This would include non-breaking changes such as typo fixes, document fixes, wording clarifications.

    v3.1.0 - The next MINOR version.

    v4.0.0 - The next MAJOR version.

@handrews
Copy link
Member Author

handrews commented Mar 5, 2024

@lornajane @AxelNennker 's comment is why I had that disclaimer... :/

Since it's approved, I'm going to merge this and then someone with more time than me can reconcile it.

@handrews handrews merged commit e9f3792 into OAI:main Mar 5, 2024
@handrews handrews deleted the dev branch March 5, 2024 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants