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.

Sign up for GitHub

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

Jump to bottom

added some explanation for usage and a commenting etiquette section. #9

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

added some explanation for usage and a commenting etiquette section. #9

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,26 @@
# obc_software_nextgen_test
Next generation linux based OBC software
Fprime software project to be ran on an embedded linux OS
Copy link

Choose a reason for hiding this comment

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

nit: should 'ran' be 'run'? (caveat: I'm no English expert)


 

## Usage
It is recommended that you use the devcontainer I have setup here: https://github.com/AlbertaSat/CUBICS_Fprime_Devcontainer. Using this docker container will ensure all contributing members have a very similar dev environment, despite their host machine configuration.

**For now the project does not include a copy of the fprime library.** You will have to clone the https://github.com/nasa/fprime repo yourself. Be sure that you modify the settings.ini framework_path to point to it.

 

### Use without a python venv
For now this project is setup without a python virtual env, which is used throughout the provided NASA tutorials. Given a docker container has been setup this seems redudant but may change in the future. In order to use the fprime-tools (fprime-util fprime-gds etc..) you will need to install the extra necessary python libraries found in fprime/requirements.txt file using pip with the following command:
```
pip -r {location_to_fprime_dir}/fprime/requirements.txt
```

The docker container also adds /home/$USER/.local/bin to your PATH. If you are not using it you must do this. This can be done with the following line:
```
echo 'export PATH=$PATH:$HOME/.local/bin' >> /home/$USERNAME/.bashrc
```


 

Expand All @@ -18,8 +39,14 @@ The following is a list of possible branch types:
- documentation

### Branching etiquette
- Branches must implement/change only a single feature.Changes related to that feature across different layers of code is acceptable.
- Branches must implement/change only a single feature. Changes related to that feature across different layers of code is acceptable.
- You may create branches off branches, but they will be merged in the order they were created. You also need to be prepared to accept the risk of having to rebase to a new parent branch parent branch, should those changes be declined.
Copy link

Choose a reason for hiding this comment

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

nit: 'parent branch' is repeated
More importantly, it may not be possible to guarantee that branches are merged in chronological order, which you point out in this sentence. It might be easier to require that branches are independent, i.e. the merge order doesn't matter. This may also make it easier to enforce a best practice: that no single branch (PR) can break the build or tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why would we not be able to enforce that branches are merged in chronological order? I agree with making branches independent to prevent from breaking builds and tests, but how would we handle the situation where branches are created off other branches. Or would this just not be allowed?

Copy link

Choose a reason for hiding this comment

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

The only reason you need to create a branch from a branch is because the new branch has a dependency on the underlying branch. That means that you likely can't make a PR from the new branch, because it will break the build until the underlying branch is merged. What usually happens is you make the PR for the new branch once the underlying branch PR is merged. So it's not that branches off branches aren't allowed, it's that breaking the build is not allowed.

- If you discover a bug unrelated to the feature you are working on, switch back to master, make a branch to fix the feature, then make a PR to merge that Bugfix. Chances are you aren't the only one experiencing the bug. You may rebase your development branch off the Bugfix branch.
Copy link

Choose a reason for hiding this comment

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

nit: make a branch to fix the 'bug', not 'feature'

- Branches that do not describe what they are changing will not be accepted.


### Commenting etiquette
- Good code should explain itself
- As we learn and improve the code for this repo it would be best if non obvious classes and functions were included with a brief explanation of how they work from a high level.
- Given we are a student organization its best if the code we write can be easily and quickly understood by those of varying skill levels and backgrounds. That being said if you think something should be commented, then comment it.