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

streamline make clean list maintenance #3256

Merged
merged 3 commits into from
Sep 12, 2022
Merged

streamline make clean list maintenance #3256

merged 3 commits into from
Sep 12, 2022

Conversation

Cyan4973
Copy link
Contributor

@Cyan4973 Cyan4973 commented Sep 7, 2022

When creating a new Makefile target to build,
it's also necessary to update the clean target,
which purpose is to remove built targets when they are present.

This process is simple, but it's also easy to forget : since there is a large distance between the position in the Makefile where the new built target is added, and the place where the list of files to clean is defined. Moreover, the list of files becomes pretty long over time, hence it's difficult to visually ensure that all built targets are present there, or that no old target (no longer produced) is no longer in the list

This PR tries to improve this process by adding a CLEAN variable. Now, when a new built target is added to the Makefile, it should preceded by :

CLEAN += newTarget
newTarget:
<TAB> ...recipe...

This new requirement is somewhat similar to .PHONY: newTarget for non-built targets.

This new method offers the advantage of locality : there is no separate place in the file to maintain a list of files to clean. This makes maintenance of make clean easier.

Also :
changed valgrindTest into test-valgrind,
to follow the same convention of all other test targets defined in tests/Makefile.

When creating a new `Makefile` target to build,
it's also necessary to update the `clean` target,
which purpose is to remove built targets when they are present.

This process is simple, but it's also easy to forget :
since there is a large distance between the position in the `Makefile` where the new built target is added,
and the place where the list of files to `clean` is defined.
Moreover, the list of files becomes pretty long over time,
hence it's difficult to visually ensure that all built targets are present there,
or that no old target (no longer produced) is no longer in the list

This PR tries to improve this process by adding a CLEAN variable.
Now, when a new built target is added to the `Makefile`,
it should preceded by :
```
CLEAN += newTarget
newTarget:
<TAB> ...recipe...
```

This new requirement is somewhat similar to `.PHONY: newTarget` for non-built targets.

This new method offers the advantage of locality :
there is no separate place in the file to maintain a list of files to clean.
This makes maintenance of `make clean` easier.
Copy link
Contributor

@terrelln terrelln left a comment

Choose a reason for hiding this comment

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

Should we also do this for the other makefiles?

@Cyan4973
Copy link
Contributor Author

Cyan4973 commented Sep 9, 2022

Should we also do this for the other makefiles?

Probably,
I wanted to have the principles reviewed before generalizing.

@Cyan4973 Cyan4973 merged commit 484b9d6 into dev Sep 12, 2022
@Cyan4973 Cyan4973 deleted the clean branch January 13, 2023 04:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants