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

rfc: init rfcs branch #551

Merged
merged 10 commits into from
Aug 12, 2024
Merged

rfc: init rfcs branch #551

merged 10 commits into from
Aug 12, 2024

Conversation

mkrainiuk
Copy link
Contributor

Description

PR introduces a process for RFCs to ensure that the community has the opportunity to comment and contribute to changes, features and other proposals for the project.

The new branch rfcs will be used for the RFC process description, template for design document, and all future design documents.

Since the PR cleanups all project files before adding new process description and template I highly recommend to review the branch https://github.com/mkrainiuk/oneMKL/tree/rfcs instead of diff.

Fixes uxlfoundation/open-source-working-group#33

Checklist

All Submissions

  • Do all unit tests pass locally? Attach a log. N/A
  • Have you formatted the code using clang-format? N/A

@mkrainiuk mkrainiuk requested a review from a team August 7, 2024 18:35
Copy link
Contributor

@andrewtbarker andrewtbarker left a comment

Choose a reason for hiding this comment

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

Looks fine to me. Do we need to change rules for CI on this branch?

@mkrainiuk
Copy link
Contributor Author

Looks fine to me. Do we need to change rules for CI on this branch?

Good point, I think we need to disable CI for this branch.

@dnhsieh-intel
Copy link
Contributor

Thanks for the PR! I just have some minor comments. Bear with me that I use copied code below. GitHub was slow when I commented on the files probably due to the large number of changed files, and permalinks didn't seem to work...

  • |Date       |Revision| Comments                                                                 |
    |-----------|--------|--------------------------------------------------------------------------|
    |  YYYYMMDD |  1.0   | Initial version                                                          |
    |  YYYYMMDD |  X.Y   |                                                                          |
    
    Do we have guidelines on when to increment X and Y? Actually, do we need X.Y, or just X is sufficient?

  • ## RFC Ratification Process
    
    1. Add new design document as a PR to this repository
        * Please add a link to preview document in the PR description,
    e.g. link for this README in your fork will be
    ```
    https://github.com/<USERNAME>/oneMKL/blob/rfcs/README.md
    ```
    
    Indenting the link might help readability, similarly for the other link at the bottom of the page.
  1. Add new design document as a PR to this repository
    • Please add a link to preview document in the PR description, e.g. link for this README in your fork will be
      https://github.com/<USERNAME>/oneMKL/blob/rfcs/README.md
      

  • 4. Organize offline review or an architecture meeting in order to collect feedback.
    
    Do we expect RFCs from external contributions?

@zettai-reido
Copy link

  • |Date       |Revision| Comments                                                                 |
    |-----------|--------|--------------------------------------------------------------------------|
    |  YYYYMMDD |  1.0   | Initial version                                                          |
    |  YYYYMMDD |  X.Y   |                                                                          |
    
    Do we have guidelines on when to increment X and Y? Actually, do we need X.Y, or just X is sufficient?

There should be no guideline but common sense.
Seeing 1.x -> 2.0 shift RFC reader expects that something was changed dramatically.
Say, APIs 1.x and 2.x are incompatible.

In world of IETF , they usually produce new RFC number altogether in this case.

For example this can be used as guideline

Major version update A revision of a specification that breaks backward compatibility
with the previous revision of the specification in numerous
significant ways.

Minor version update A revision of a specification that may add or enhance
functionality, fix bugs, and make other changes from the previous
revision, but the changes have minimal impact, if any, on
backward compatibility.

@mkrainiuk
Copy link
Contributor Author

Thanks @dnhsieh-intel!

  • Do we have guidelines on when to increment X and Y? Actually, do we need X.Y, or just X is sufficient?

I think we need both as Andrey mentioned we can expect minor changes or significant changes in the proposal, but I can slightly change the naming to make it more clear, e.g. X.Y -> major.minor will it be better? Or it's better to add full guideline for the versioning suggested by @zettai-reido?

Indenting the link might help readability, similarly for the other link at the bottom of the page.

Good point, updated.

Do we expect RFCs from external contributions?

Yes, as opensource project we can expect proposals from any user, so probably "architecture" meeting is not the right name for it, we have opensource forum and SIG WG, I can add them as potential forums where we can review the RFC online.

@dnhsieh-intel
Copy link
Contributor

I think we need both as Andrey mentioned we can expect minor changes or significant changes in the proposal, but I can slightly change the naming to make it more clear, e.g. X.Y -> major.minor will it be better? Or it's better to add full guideline for the versioning suggested by @zettai-reido?

I would prefer we include full guidelines. I don't want to assume that everyone automatically interprets major.minor the same way. If we don't write them down, we might have different systems sooner or later.

Yes, as opensource project we can expect proposals from any user, so probably "architecture" meeting is not the right name for it, we have opensource forum and SIG WG, I can add them as potential forums where we can review the RFC online.

Good idea!

@mkrainiuk
Copy link
Contributor Author

I've update template and readme with the required changes, let me know if you have any other suggestions how to improve the process or template.

Copy link
Contributor

@dnhsieh-intel dnhsieh-intel left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! We might need to use ** in the workflows just in case. Please see filter pattern.

@mkrainiuk mkrainiuk merged commit 9c2abfa into oneapi-src:rfcs Aug 12, 2024
@rodburns
Copy link

Looks good. Where will the process description live, it doesn't appear in the PR diff?
Would it make sense to have a folder for each RFC in case there are attachments or other supporting files (source files perhaps)?

@mkrainiuk
Copy link
Contributor Author

Hi @rodburns,

Where will the process description live, it doesn't appear in the PR diff?

I'm going to add the link to new branch in the second PR #555 that is waiting for one more approval.

Would it make sense to have a folder for each RFC in case there are attachments or other supporting files (source files perhaps)?

This is part of document-style, the recommendation is to create rfcs/<YYYMMDD>-descriptive-but-short-proposal-name/ folder, create README.md as the main document and add any supporting files to the same folder.

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.

5 participants