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

Display pkg_config in test logs #134

Merged
merged 4 commits into from
Feb 6, 2024
Merged

Display pkg_config in test logs #134

merged 4 commits into from
Feb 6, 2024

Conversation

hdholm
Copy link
Contributor

@hdholm hdholm commented Jun 2, 2023

I don't know if this will help anyone else. During all the pkg_config changes for PR #98 it was difficult to tell what effects changes where having. There doesn't seem to be a good way to "test" pkg_config as part of github actions (I think you'd have to install and then try and install something like tang on top of it.) But this tiny patch at least displays what pkg_config looks like so it's relatively easy to at least see what changed.

Path from test and show tests slightly different
Copy link
Collaborator

@sarroutbi sarroutbi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. As we are executing cat command on a file, I suggest testing the file existence before doing the cat command so that if the file does not exist, failure doesn't take place

@hdholm
Copy link
Contributor Author

hdholm commented Jun 6, 2023

@sarroutbi I don't have strong feelings one way or the other, but I think there is an argument to be made that if the pkg_config file doesn't exist it should be an error worth investigating.

@sarroutbi sarroutbi self-requested a review February 6, 2024 14:25
@sarroutbi sarroutbi merged commit 5e45732 into latchset:master Feb 6, 2024
23 checks passed
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.

3 participants