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

Do not run CI on documentation changes #2532

Merged
merged 1 commit into from
Mar 23, 2022

Conversation

ann0see
Copy link
Member

@ann0see ann0see commented Mar 20, 2022

Short description of changes
Stops running the CI if the documentation changes. There is no need of running the CI on non source files. New version of #1921 I think this is a quick fix (and can be update ofc.)

CHANGELOG: Stop running Autobuild if only documentation is updated to avoid wasting computation time.

Context: Fixes an issue?
No. Related to #2282 (comment)

Does this change need documentation? What needs to be documented and how?
No

Status of this Pull Request
Ready for review

What is missing until this pull request can be merged?

Checklist

@ann0see
Copy link
Member Author

ann0see commented Mar 20, 2022

Peter‘s alternative approach might also work, but this is faster to implement.

Copy link
Member

@hoffie hoffie left a comment

Choose a reason for hiding this comment

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

Seems safe to do.

Note: README.md is part of Jamulus.pro's DISTFILES, so I guess it's part of make dist, but we don't run that on CI.

paths-ignore doesn't work for tags per the docs, so even if we ignore too much, we should be on the safe side for releases.

The duplication is not nice, but I don't see a way around.
Doing trigger-based ignores has the advantage of being able to still run the Action manually if this logic is wrong for some reason.

@hoffie
Copy link
Member

hoffie commented Mar 20, 2022

Maybe I'm the only one caring about git log cleanliness, but I'd prefer context in the commit message.
Autobuild: Don't run on README/docs-only commits (the current message isn't clear that it's about the auto build logic)

@softins
Copy link
Member

softins commented Mar 21, 2022

Just wondering, what about ChangeLog? And are the other *.md files in the root dir going to move into docs/ at some point?

@ann0see ann0see force-pushed the feature/noRunDocs branch 2 times, most recently from bf50209 to 890e277 Compare March 21, 2022 20:17
@ann0see
Copy link
Member Author

ann0see commented Mar 21, 2022

Hmm. I've now added other files (except for the ChangeLog since it's somehow related to the release process)

@pljones
Copy link
Collaborator

pljones commented Mar 22, 2022

Just wondering, what about ChangeLog? And are the other *.md files in the root dir going to move into docs/ at some point?

ChangeLog, README.md and any licence-related files should stay at root level, otherwise discovery proves more difficult. (Particularly auto-discovery.)

@ann0see
Copy link
Member Author

ann0see commented Mar 22, 2022

Probably true 😀. Not sure what the gold standard is.

@softins
Copy link
Member

softins commented Mar 22, 2022

Just wondering, what about ChangeLog? And are the other *.md files in the root dir going to move into docs/ at some point?

ChangeLog, README.md and any licence-related files should stay at root level, otherwise discovery proves more difficult. (Particularly auto-discovery.)

Sorry, I conflated two things in my comment:

  • should ChangeLog be included in the files that do not trigger a rebuild?
  • should the other *.md files except README.md be moved into docs/?

@pljones
Copy link
Collaborator

pljones commented Mar 22, 2022

I think all the remaining top-level docs need to stay there.

  • APPLEAPPSTORE.LICENCE.WAIVER and COPYING are licence-related
  • README.md is definitely needed there
  • Anyone looking for COMPILING.md shouldn't have to look to far - it's a "first place of reference" for those who are doing it themselves
  • SECURITY.md and CONTRIBUTING.md could perhaps move unless used by Github for "project completeness scoring" or whatever
  • Jamulus.entitlements - no idea what this is or why it's at top level.

@ann0see
Copy link
Member Author

ann0see commented Mar 22, 2022

should ChangeLog be included in the files that do not trigger a rebuild?

I excluded it for the reason that I think the Debian build uses it and it’s related to the release process

@ann0see
Copy link
Member Author

ann0see commented Mar 22, 2022

Jamulus.entitlements - no idea what this is or why it's at top level.

#2529

@ann0see ann0see force-pushed the feature/noRunDocs branch from 890e277 to 13a16f0 Compare March 22, 2022 22:02
@ann0see ann0see merged commit 7610239 into jamulussoftware:master Mar 23, 2022
@ann0see ann0see deleted the feature/noRunDocs branch March 23, 2022 19:15
@pljones pljones added this to the Release 3.9.0 milestone Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants