-
Notifications
You must be signed in to change notification settings - Fork 11
Add evaluation guidelines and instructions on how to submit a project #6
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
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
# Evaluation guidelines | ||
|
||
These are the guidelines used by the Resources team to evaluate project | ||
submissions. | ||
|
||
## Hard requirements | ||
|
||
The project must fulfill these requirements or it will be rejected. | ||
|
||
- The main logic of the application must be written in Rust. | ||
|
||
- The source code must be public. Licensing terms are unimportant. | ||
|
||
## Bonus points | ||
|
||
> N.B. A project repository may contain more than one crate. In those cases, one | ||
> (binary) crate will be the application, and the other crates will be libraries | ||
> and / or tools. In the review consider only the crates that are compiled for | ||
> the target device. One of these crates will be the application ("application | ||
> code") and the rest will be libraries ("support code"). | ||
|
||
Award points if | ||
|
||
- The project is unlike the projects currently on display. | ||
|
||
- The documentation includes instructions on how to build the program, or the | ||
firmware can be built with just `cargo build`. | ||
|
||
- The repository has a CI setup. | ||
|
||
- The application compiles on stable. | ||
|
||
- The application code is free from `unsafe` code and all `unsafe` code has been | ||
pushed to libraries that only expose a safe API. | ||
|
||
- Code that's not target specific, if any, has unit tests. | ||
|
||
- Support code, if any, is documented. | ||
|
||
## Penalties | ||
|
||
Subtract points if | ||
|
||
- The application contains potential soundness issues. For example, using | ||
`mem::transmute`, unchecked creation of singletons and unchecked use of | ||
`static mut` variables are huge red flags. | ||
|
||
- Explicit panicking (e.g. `unwrap`) is used instead of proper error handling. | ||
However, note that `unreachable!` is OK if properly used. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,37 +1,44 @@ | ||
# Project name | ||
# NOTE If submitting a new project add it to the end of the list, before the template | ||
|
||
# Dummy entry | ||
- name: Lorem Ipsum | ||
website: https://example.org | ||
author: Alice | ||
author_website: https://github.com/ghost | ||
description: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu pharetra purus. Sed laoreet lectus ut convallis molestie. Etiam suscipit vitae ante a finibus. Fusce aliquet viverra efficitur. Curabitur sit amet tellus risus." | ||
image: https://via.placeholder.com/900x675 | ||
|
||
# Project website | ||
# Another dummy entry | ||
- name: Ipsum Lorem | ||
website: https://example.org | ||
author: Bob | ||
author_website: https://github.com/ghost | ||
description: "Vivamus varius cursus aliquam. Praesent scelerisque, neque efficitur luctus pharetra, augue urna tempus lectus, at ullamcorper sem purus et enim. Nam urna eros, ornare quis mi posuere, aliquam egestas turpis. Suspendisse quis mi viverra, malesuada justo ullamcorper, blandit risus" | ||
image: https://via.placeholder.com/900x675 | ||
|
||
# Template for new entries | ||
|
||
# Project name | ||
# - name: Lorem Ipsum | ||
|
||
# Project website | ||
# website: https://example.org | ||
|
||
# Author name or username | ||
author: Alice | ||
# author: Alice | ||
|
||
# Author's GitHub / Twitter profile or personal website | ||
author_website: https://github.com/ghost | ||
# author_website: https://github.com/ghost | ||
|
||
# SHORT description of the project (max 280 characters) | ||
description: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu pharetra purus. Sed laoreet lectus ut convallis molestie. Etiam suscipit vitae ante a finibus. Fusce aliquet viverra efficitur. Curabitur sit amet tellus risus." | ||
# description: "Lorem ipsum dolor sit amet, consectetur adipiscing elit. Donec eu pharetra purus. Sed laoreet lectus ut convallis molestie. Etiam suscipit vitae ante a finibus. Fusce aliquet viverra efficitur. Curabitur sit amet tellus risus." | ||
|
||
# Image of the project | ||
# if not a URL, place the file in the `assets` folder | ||
image: https://via.placeholder.com/900x675 | ||
# image: https://via.placeholder.com/900x675 | ||
|
||
# OR a video of the project | ||
# multiple video formats can be supplied | ||
# if not a URL, place the file in the `assets` folder | ||
# video: | ||
# - ipsum.mp4 | ||
|
||
- name: Ipsum Lorem | ||
|
||
website: https://example.org | ||
|
||
author: Bob | ||
|
||
author_website: https://github.com/ghost | ||
|
||
description: "Vivamus varius cursus aliquam. Praesent scelerisque, neque efficitur luctus pharetra, augue urna tempus lectus, at ullamcorper sem purus et enim. Nam urna eros, ornare quis mi posuere, aliquam egestas turpis. Suspendisse quis mi viverra, malesuada justo ullamcorper, blandit risus" | ||
|
||
# if not a URL, place the file in the `assets` folder | ||
image: https://via.placeholder.com/900x675 |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Uhm, what is an explicit panic? There're valid uses of
unwrap()
and in many cases there's simply no other way of properly handling an error other than to panic.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 guess we could ask for
expect("bla bla")
instead ofunwrap()
which at least gives a reason for the panic, though there might be binary size concerns...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.
unwrap
,expect
, etc.Compare:
Explicit panicking
vs
Proper error handling (robust code)
IMO,
unwrap
should only be used when you know the code won't panicat runtime. For example,
Resut::<T, !>.unwrap
is OK because there's noalternative API.
IMO, jumping to a "fail-safe" state when unrecoverable I/O errors happen is preferable
over panicking. Panicking should only happen due to bugs (programmer errors /
contract violations), which should be fixed during development phase. Handling
both cases in the same way makes it hard to spot real bugs.
I can reword this line as "Explicit panicking (e.g.
unwrap
) is used whereproper error handling is possible.", but the bottom line here is that explicit
panicking is a "code smell" and demands a closer inspection of the code.
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.
Sure a failsafe would be preferable but that may not always be available, also for some applications halting is a no-go and a reset is preferred instead. There're are other classes or errors which can happen for which they might not be a useful handling procedure like if there're pluggable components and those are not present but required for operation.
Everything here has a big dependency on the application and the hardware and IMHO there's neither a one fits all nor a good/bad classification possible. I'd rather encourage finding creative ways to avoid panicking rather than penalisation. Also instead of
unwrap
ping an irrelevantResult
I'd rather suggest usingok()
.