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 journald logging datatypes on non-Linux #37

Closed
wants to merge 2 commits into from

Conversation

matthewbauer
Copy link
Collaborator

These need to be available in some cases.

These need to be available in some cases.
This is needed for release.nix to properly build things on another
system.
Ericson2314 referenced this pull request Feb 5, 2019
@3noch
Copy link
Collaborator

3noch commented Feb 5, 2019

Can we just use #7 instead?

@3noch
Copy link
Collaborator

3noch commented Feb 5, 2019

IIRC depending on journald in cabal prevents mac support so using a cabal flag as #7 does allows you to choose this at the dependency level.

@Ericson2314
Copy link
Member

As I commented on 345a0e2, I think that turning of a compiler error into a runtime error was a bad idea, and that was the domino that necessitates this. @3noch's proposal to be static and explicit (say what you mean in the CPP) is I think the right approach, and in the opposite direction.

@matthewbauer
Copy link
Collaborator Author

matthewbauer commented Feb 5, 2019

I don't see how introducing a cabal flag improves the situation besides making things more confusing. The issue was introduced in 345a0e2. I guess it's debatable whether it should be a runtime error or a type error though... But if it's a runtime error, this is needed to get things working again.

@ali-abrar
Copy link
Member

Are users ever expected to write code that uses this constructor? If so, I'd rather not force CPP into user code, but if not we can revert 345a0e2 and replace with another approach.

@ali-abrar
Copy link
Member

What happened to this commit, I wonder: 9146836 @luigy

@3noch
Copy link
Collaborator

3noch commented Feb 5, 2019

Haha we've implemented the same thing multiple times!

@ali-abrar
Copy link
Member

Closing in favor of #38

@ali-abrar ali-abrar closed this Feb 5, 2019
@ali-abrar ali-abrar deleted the add-journald-types branch November 20, 2021 19:01
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