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

Add make help #3430

Merged
merged 7 commits into from
Feb 14, 2025
Merged

Add make help #3430

merged 7 commits into from
Feb 14, 2025

Conversation

dlebauer
Copy link
Member

@dlebauer dlebauer commented Feb 4, 2025

The goal is to make it easier to use PEcAn's make functionality by adding documentation.

This PR

  • Adds help target to enable make help
  • Documents all targets and added helpful info

TODO:

  • Update documentation
  • Update changelog

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My change requires a change to the documentation.
  • My name is in the list of CITATION.cff
  • I agree that PEcAn Project may distribute my contribution under any or all of
    • the same license as the existing code,
    • and/or the BSD 3-clause license.
  • I have updated the CHANGELOG.md.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Add help target to enable make help
Documented all targets and added helpful info
Makefile Outdated
Comment on lines 113 to 114
@echo " - Standard workflow: install packages, run checks, test, and document before submitting a PR."
@echo " - Before submitting a PR, please ensure that all tests pass, code is linted, and documentation is up-to-date."
Copy link
Member

Choose a reason for hiding this comment

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

I think this is intended to be normative ("Your standard workflow when preparing to submit a PR should include all of installing packages, running checks, ..."), but it could also be read as a descriptive summary of what this Makefile's standard workflow does ("The effect of running the standard workflow is to install packages, run checks, ...") , raising the question of how one invokes said workflow or switches to a nonstandard one when needed.

Consider pointing to the PEcAn book for conventions on developer workflows and saying a bit more for folks in first-time installation mode who haven't yet connected that their setup process is "just" the install part of the standard dev workflow?

Copy link
Member Author

Choose a reason for hiding this comment

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

see above https://github.com/PecanProject/pecan/pull/3430/files#r1942059169 and feel free to propose a specific change

Copy link
Member

Choose a reason for hiding this comment

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

GitHub's making it hard for me to tell if you already applied the above or if it's a pending proposal, but yes it seems like an improvement.

Makefile Outdated
Comment on lines 113 to 114
@echo " - Standard workflow: install packages, run checks, test, and document before submitting a PR."
@echo " - Before submitting a PR, please ensure that all tests pass, code is linted, and documentation is up-to-date."
Copy link
Member

Choose a reason for hiding this comment

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

GitHub's making it hard for me to tell if you already applied the above or if it's a pending proposal, but yes it seems like an improvement.

@dlebauer
Copy link
Member Author

@infotroph I welcome your suggestions implemented as a PR against this branch.

@dlebauer dlebauer merged commit 56d0c4e into PecanProject:develop Feb 14, 2025
34 of 46 checks passed
@dlebauer dlebauer deleted the make_help branch February 14, 2025 02:24
@dlebauer dlebauer restored the make_help branch February 14, 2025 17:42
@dlebauer dlebauer mentioned this pull request Feb 14, 2025
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