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

Add contributing tips #2704

Merged
merged 6 commits into from
May 17, 2024
Merged

Add contributing tips #2704

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8bc5ecf
Add contributing tips
JohanBertrand Apr 28, 2024
d02028b
Sort spelling and add sbt
JohanBertrand Apr 28, 2024
8f223f7
Change fpp to fprime folder in tests run commands
JohanBertrand Apr 29, 2024
7332679
Update contributing.md
JohanBertrand May 1, 2024
5af2fa6
Update contributing readme
JohanBertrand May 8, 2024
57646a9
Merge branch 'devel' into Add-contributing-tips
LeStarch May 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions .github/actions/spelling/expect.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,13 +268,13 @@ doxyindexer
doxyrules
doxysearch
Doxywizard
DPCFG
dpi
DPCAT
DPCATALOG
DPCFG
dpi
DPMANAGER
DPWRITER
dps
DPWRITER
DRAINBUFFERS
drv
dsdl
Expand Down Expand Up @@ -433,8 +433,8 @@ Guire
handcoded
hardtoaccess
hashvalue
HEADERSIZE
hdr
HEADERSIZE
headlessly
heapifying
hexid
Expand Down Expand Up @@ -900,6 +900,7 @@ saveop
sbb
SBF
SBINDIR
sbt
scatterometer
schem
schematron
Expand Down
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ build-fprime-automatic*
/ci-venv/
/ci-logs*
/ci-Framework-logs*
/ci-Ref-logs*
TesterBase.*
GTestBase.*
**/DefaultDict/serializable/*
Expand Down
48 changes: 47 additions & 1 deletion CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ get the submission moving forward.
### Automated Checks on Reference Repositories

Some of the above-mentioned automated checks run on reference applications that are not part of the core F´ repository, such as our [tutorial repositories](https://github.com/fprime-community#tutorials). This serves two main purposes: running more tests, and making sure our suite of reference applications and tutorials do not go out-of-date.
Because of this pattern, users who submit a pull request which introduces breaking changes on _how_ F´ is used in those external repositories will need to submit associated pull requests to introduce a fix on said external repositories.
Because of this pattern, users who submit a pull request which introduces breaking changes on _how_ F´ is used in those external repositories will need to submit associated pull requests to introduce a fix on said external repositories.

The checks are configured to run on the `devel` branch of each external repository, but will prioritize the branch `pr-<PR_NUMBER>` if it exists, with `PR_NUMBER` being the number of the pull request that has been opened in nasa/fprime.

Expand Down Expand Up @@ -150,3 +150,49 @@ tools this can increase the effort required to review a submission. Be careful w
The automatic checking system will run all our unit tests and integration tests across several systems. However, this
process will take time. Try to run the unit tests locally during development before submitting a PR and use the
automatic checks as a safety net.

The tests can be run using the following commands:

```bash
# Go into your FPP directory
cd MY_FPP_DIRECTORY

# Run CI tests on fprime
./ci/tests/Framework.bash

# Run CI tests on the reference application
./ci/tests/Ref.bash

# Run the static analyzer
# Purge all directory
fprime-util purge -f
# Generate the build files for clang-tidy. Make sure clang-tidy is installed.
fprime-util generate -DCMAKE_CXX_CLANG_TIDY="clang-tidy-12;--config-file=$PWD/release.clang-tidy"
# Build fprime with the static analyzer
fprime-util build -j4
```

## Development with modified FPP version

In case FPP needs to be locally changed, FPP first needs to be installed:

```bash
# Go into your FPP directory
cd MY_FPP_DIRECTORY
# Install FPP. Make sure to have sbt installed.
./compiler/install
# Copy
cp ./compiler/bin/* /usr/local/bin/ -r
Copy link
Collaborator

Choose a reason for hiding this comment

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

I personally do

export PATH=<path/to/fpp>/compiler/bin:$PATH

Do you think it might be better to recommend that in the guide? Forgetting to uninstall could lead to very hard to debug issues

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also give the destination in the install line....and place it into your virtual environment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I definitely think that there is a better way than mine.

I feel like, ideally, there should be a setup.py that we could use to install the fpp packages with pip install . after the command compile/install has been called. I know the setup.py file existed in the past (https://github.com/nasa/fpp/blob/v2.0.1/setup.py), but I'm not sure what was the reason why it was removed.

As you suggested, the export PATH way is already better than what I am currently doing.

I have to say that I am not using any virtual environment in my development routine right now, since I am having a dedicated docker image for fprime. However, what would be the command to add it to the virtual environment?
Maybe both approaches can be suggested.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do the instructions here work: https://github.com/nasa/fpp/blob/main/compiler/README.adoc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it works. I updated the readme with the link, and added some clang-tidy commands we discussed in other PR.

Let me know if there is something else to change

```

Then, `fprime-util generate` needs to be run using `-DFPRIME_SKIP_TOOLS_VERSION_CHECK=1`

For example, to generate and build F´:
```bash
# Go into the fprime directory
cp MY_FPRIME_DIRECTORY
# Generate the build files without checking the FPP version
fprime-util generate -DFPRIME_SKIP_TOOLS_VERSION_CHECK=1
# Build the project
fprime-util build -j4
```