-
Notifications
You must be signed in to change notification settings - Fork 20
Add GOALS.md, revise contribution process, freshen up to use arewesafetycriticalyet.org #149
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
base: main
Are you sure you want to change the base?
Add GOALS.md, revise contribution process, freshen up to use arewesafetycriticalyet.org #149
Conversation
❌ Deploy Preview for scrc-coding-guidelines failed.
|
99c4170
to
863169b
Compare
@PLeVasseur do you want me to take a look at this? :) |
That'd be very kind of you ;D |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I hope you're ready for many comments <3
README.md
Outdated
|
||
For a new coding guideline you'd like to contribute, start with opening a [coding guideline issue](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/new?template=CODING-GUIDELINE.yml). | ||
|
||
Once an issue has been well-developed enough it's then time to write up the guideline. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know how I'd change this phrasing, but here's my observation: we now have an automatic PR builder after a Coding Guideline issue has been approved, right? Perhaps this description of the process could be changed, then, since it's now a different process than what is described in this line. Does that make sense?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed, this should be updated to reflect current reality 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@felix91gr -- can we call this one resolved as I took my attempt at this?
GOALS.md
Outdated
|
||
A good first step is to open a new [coding guideline issue](https://github.com/rustfoundation/safety-critical-rust-coding-guidelines/issues/new?template=CODING-GUIDELINE.yml). | ||
|
||
# Goals |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the items in # Goals
lack a verb to anchor them to the world. I think those are really important for goal-setting
- To have coding guidelines that make a "best effort" attempt (...)
- To have practical recommendations on how to (...)
- (this one is ok)
- (same)
- To have (find or create) Clippy lints which will cover decidable guidelines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very salient point
I'll visit this again
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried my hand at this; could you look again?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OH! Awesome! Yes, I shall :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's now under Criteria, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, exactly
GOALS.md
Outdated
|
||
## Goals obtained by discussion with Tooling Subcommittee | ||
|
||
* Make a label for each which _in theory_ is decidable or not |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For each... guideline, right?
(I'd make it explicit: Make a label for each guideline)
Though if we're talking about guidelines, I'd perhaps phrase it a bit differently, because the sentence feels a bit weird right now:
Make a label for each guideline, which describes whether said guideline is decidable or not.
Plus a link to our definition of decidable, or the Wikipedia article on Decidability otherwise.
Decidability is always a theoretical thing. So I think it's fine to omit the in theory bit. As long as we give the reader a link to know the fine details of it, I think that's plenty good enough :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've revised the wording on this -- could you take a look?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's a lot better now! I would still add a link to a definition of decidability tho, just so that people who need a refresher or haven't studied CS and read the text know where to go for more details :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, have done so now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestions are now in place ^^
…wesafetycriticalyet.org
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
59cb10c
to
4f0f9fe
Compare
7f7e285
to
7c89bd9
Compare
46d94f8
to
494cbda
Compare
494cbda
to
ff367d1
Compare
GOALS.md
Outdated
|
||
## Elevator pitch | ||
|
||
We will make Rust coding guidelines are available within this repository, deployed to an accessible location on the internet which comply with relevant standards for various safety-critical industries such as: IEC 61508, ISO 26262, and DO 178. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something about this sentence is missing, I believe.
Was it intended to go like:
We will make sure Rust coding guidelines are available within this repository, (...)
or perhaps something along the lines of...
We will make Rust coding guidelines. Such guidelines are available within this repository, (...)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, one of those two, thanks for catching the phrasing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, cleaned this up into three sentences instead so it's a bit easier to comprehend.
GOALS.md
Outdated
* We aim to produce evidence-based guidelines, with statistics around human error when programming Rust, to support: | ||
1. What guidelines are written, and | ||
2. Why a specific suggestion was made | ||
* We will produce the guidelines in an artifact that's easily machine readable and consistent format to make it easier to consume by tool vendors as a baseline (e.g. multiple JSON files, one per language piece, also potentially one large JSON concatenated together) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean here by "as a baseline"? :o
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay. I guess the phrasing here may not be clear. The idea is that we will satisfy the Tooling Subcommittee's ask by providing these JSON files. If they need more they can come back and we can discuss.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll take another pass to clarify
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Clarified what exactly we're producing today
## Criteria | ||
|
||
* We produce coding guidelines that make a "best effort" attempt at cataloging common pieces (e.g. functions, arithmetic, unsafe) of the Rust programming language and how they fit into a safety-critical project | ||
* We will use [MISRA Compliance: 2020](https://misra.org.uk/app/uploads/2021/06/MISRA-Compliance-2020.pdf) for categorization |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which part of the MISRA Compliance: 2020 document are we using? (the pdf is 39 pages long)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
scary voice: the whole thing!!
But seriously, @AlexCeleste may be better equipped to tell us if we need cite a page range or the entire document.
README.md
Outdated
## Contributing to the coding guidelines | ||
|
||
See [CONTRIBUTING.md](CONTRIBUTING.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this entire section should go inside of CONTRIBUTING.md
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is getting long, isn't it? I could see the argument
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, extracted this out
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Co-authored-by: Félix Fischer <[email protected]>
Closes #145