Skip to content

Install syntax files #2187

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
Merged
merged 2 commits into from
Apr 22, 2025
Merged

Install syntax files #2187

merged 2 commits into from
Apr 22, 2025

Conversation

SFrijters
Copy link
Contributor

@SFrijters SFrijters commented Apr 21, 2025 •
edited
Loading

I've been updating the conky derivation in nixpkgs and in the process I've also added support for the BUILD_EXTRAS flag.
However, I noticed that the generated syntax files are not installed, so they are basically "lost" inside the Nix build sandbox unless I move them manually. I think it would be reasonable to just install them by default.

Tested by using this branch as the source for the updated nixpkgs derivation.

I'm not sure if this needs a doc update?

Checklist

  • I have described the changes
  • I have linked to any relevant GitHub issues, if applicable
  • Documentation in doc/ has been updated
  • All new code is licensed under GPLv3

Description

  • Describe the changes, why they were necessary, etc
  • Describe how the changes will affect existing behaviour.
  • Describe how you tested and validated your changes.
  • Include any relevant screenshots/evidence demonstrating that the changes work and have been tested.

@github-actions github-actions bot added documentation suggests documentation changes or improvements extras suggests addition or modification of files that ease integration with external software labels Apr 21, 2025
@netlify Netlify
Copy link

netlify bot commented Apr 21, 2025 •
edited
Loading

✅ Deploy Preview for conkyweb canceled.

Name Link
🔨 Latest commit 3700a9a
🔍 Latest deploy log https://app.netlify.com/sites/conkyweb/deploys/68075b6b54bd150008c4c21b

Caellian
Caellian requested changes Apr 22, 2025
View reviewed changes
Copy link
Collaborator

@Caellian Caellian left a comment

Choose a reason for hiding this comment

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

A minor tweak to simplify configuration for packagers.

Other than that, looks good. Nice catch with unnecessary BUILD_DOCS guard.

@Caellian Caellian added enhancement suggests alteration of existing functionality to better support different use cases packaging related to conky packaging, package distribution or installation labels Apr 22, 2025
@SFrijters
Install generated syntax files
316de8d
@SFrijters
Remove redundant if in CMake
3700a9a 
This is already in an `if(BUILD_DOCS)` block.
@SFrijters
Copy link
Contributor Author

Thanks for the quick review - the requested changes have been applied.

@SFrijters SFrijters requested a review from Caellian April 22, 2025 09:04
@brndnmtthws brndnmtthws merged commit 21996a3 into brndnmtthws:main Apr 22, 2025
30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brndnmtthws brndnmtthws brndnmtthws approved these changes

@Caellian Caellian Awaiting requested review from Caellian

Assignees
No one assigned
Labels
documentation suggests documentation changes or improvements enhancement suggests alteration of existing functionality to better support different use cases extras suggests addition or modification of files that ease integration with external software packaging related to conky packaging, package distribution or installation
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@SFrijters @brndnmtthws @Caellian