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

Minor Makefile cleanup #40

Merged
merged 1 commit into from
Jul 1, 2024

Conversation

michael-grunder
Copy link
Collaborator

  • We can specify SHELL = /bin/sh to avoid having to do $(shell sh -c '<cmd>') and instead just doing $(shell <cmd).
  • Use makefile built-in word to parse library version info.
  • Minor cleanup of OS specific conditional blocks.
  • Remove dead CXX assignment (it was required for the qt example).
  • re-enable -pedantic for FreeBSD

@michael-grunder michael-grunder requested a review from bjosv June 30, 2024 03:47
* We can specify SHELL = /bin/sh to avoid having to do `$(shell sh -c
  '<cmd>')` and instead just doing `$(shell <cmd)`.
* Use makefile built-in `word` to parse library version info.
* Minor cleanup of OS specific conditional blocks.
* Remove dead `CXX` assignment (it was required for the qt example).
* re-enable `-pedantic` for FreeBSD

Signed-off-by: michael-grunder <[email protected]>
Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

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

LGTM!
If we keep SunOS support its might be good that we look into a CI for it (for a rainy day).
Maybe https://github.com/vmactions/solaris-vm can be used.

@michael-grunder michael-grunder merged commit 90f6b97 into valkey-io:main Jul 1, 2024
40 of 43 checks passed
@michael-grunder michael-grunder deleted the makefile-cleanup branch July 1, 2024 18:01
@michael-grunder
Copy link
Collaborator Author

If we keep SunOS support its might be good that we look into a CI for it (for a rainy day)

Good idea. I know people use Hiredis in some exotic environments (like AIX).

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.

2 participants