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

Added rfc for removing vulkan SDK. #655

Open
wants to merge 1 commit into
base: rfcs
Choose a base branch
from

Conversation

coldav
Copy link
Collaborator

@coldav coldav commented Jan 23, 2025

Overview

Added rfc for removing vulkan SDK.

Reason for change

See rfc

Description of change

See rfc

rfc-0004.md Outdated
Comment on lines 1 to 8
---
rfc: 4
title: Removing support for Vulkan
author: Colin Davidson <[email protected]>
status: draft
---

``` text
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
---
rfc: 4
title: Removing support for Vulkan
author: Colin Davidson <[email protected]>
status: draft
---
``` text

The idea was that https://uxlfoundation.github.io/oneapi-construction-kit/rfc-0002.html showed what should be used as a template for new files, but you have taken the whole rfc-0002.md as the template for new files. This may have been unclear in the process, thoughts welcome on how to make this clearer.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm a little unclear on why we should remove these lines? They seemed to give useful information.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the first six lines, this is useful but it is duplicated, this is already present in the immediately following bit. For the eighth line, it means the whole RFC is treated as plain text rather than Markdown.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, of course you are quite right, I should have noticed the intention. Personally I prefer a template I can just copy as is.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I get that. The background here is that the inspiration for this, https://peps.python.org/pep-0012/, contains a lot more than just the template, it also contains details on how to write RST. That documentation would not apply to us since we use Markdown, so I took it out for us, and the template is all that is left. Ideally, in the future, we would add some Markdown documentation there.

@coldav coldav force-pushed the colin/add_vulkan_rfc branch from 635580b to 21b3fd9 Compare January 24, 2025 15:30
rfc-0004.md Outdated
@@ -0,0 +1,85 @@
---
rfc: 0004
Copy link
Collaborator

@hvdijk hvdijk Jan 24, 2025

Choose a reason for hiding this comment

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

Suggested change
rfc: 0004
rfc: 3

No need for the leading 0s outside of the file name, including them here would cause them to show up on the index page, and the first available RFC number is 3. (This should of course also be accompanied by a rename of the file.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure, renamed as well.

@coldav coldav force-pushed the colin/add_vulkan_rfc branch from 21b3fd9 to 94c69a9 Compare January 24, 2025 15:50
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.

2 participants