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

Haddocks-checking of test suites and benchmarks seems unnecessary #605

Closed
sjakobi opened this issue Aug 9, 2022 · 12 comments
Closed

Haddocks-checking of test suites and benchmarks seems unnecessary #605

sjakobi opened this issue Aug 9, 2022 · 12 comments

Comments

@sjakobi
Copy link
Contributor

sjakobi commented Aug 9, 2022

I'm seeing some weird CI failures after regenerating a GitHub Actions config with the current haskell-ci master: haskell-unordered-containers/unordered-containers#477

Apparently the enabling of the --haddock-all option (via #585) means that incorrect haddock syntax in benchmarks now cause CI failures.

Frankly I don't understand the purpose of these checks – I don't think anyone cares about the haddocks of benchmark components.

So I'd like to request that haskell-ci doesn't check the haddocks of test-suite and benchmark components by default.

@phadej
Copy link
Collaborator

phadej commented Aug 9, 2022

Discuss with @andreasabel, he added --haddock-all.

EDIT: See discussions in #582

@andreasabel
Copy link
Contributor

I think that checking all haddocks is a good default, but one should be able to opt out of this. We'll likely need a new configuration field for that.

@sjakobi
Copy link
Contributor Author

sjakobi commented Aug 12, 2022

We'll likely need a new configuration field for that.

How about a flag haddock-all to toggle cabal's --haddock-all option?

@andreasabel
Copy link
Contributor

Or haddock-what with possible monoidal values all (default), executables etc.

@sjakobi
Copy link
Contributor Author

sjakobi commented Aug 12, 2022

Both would be fine for me. Of course, I still think that by default, the haddock steps should be limited to the libraries.

@phadej
Copy link
Collaborator

phadej commented Aug 21, 2022

I found a good reason to revert the --haddock-all change: modules with inspection-testing tests fail miserably when you try to haddock them.

I'm waiting two weeks for a patch EDIT: for an additional config option (either allowing to simply restricting haddock to libraries only, or more granular component selection), otherwise I'll revert 2922aa5

@andreasabel
Copy link
Contributor

Im a fine with a revert. I can patch in the haddock step in the hackage-server CI with a bit of extra work.
I still think that the haddock step should be forced if there is a haddock configuration in the cabal.haskell-ci file even if there is no library in the project. But this might be harder to implement if haddock: True isn't distinguished from no haddock: field.

@phadej
Copy link
Collaborator

phadej commented Aug 21, 2022

I was probably unclear, I'll welcome a separate config option (say haddock-all: True/False or haddock-components: all|lib|lib exes|...). But as --haddock-all is blocking upgrade and I'm short on time myself, I'll be forced to revert --haddock-all patch instead of implementing above additional feature myself.

@andreasabel
Copy link
Contributor

Yes, understood, thanks for the clarification.
Atm, I cannot dive into adding this option, so I approve reversal.

RyanGlScott added a commit to goldfirere/singletons that referenced this issue Aug 22, 2022
This is required until a fix for haskell-CI/haskell-ci#605 lands upstream.
@RyanGlScott
Copy link
Contributor

RyanGlScott commented Aug 22, 2022

I've also ran into another situation in singletons' CI where --haddock-all can trigger a cabal bug—see haskell/cabal#5423. I am working around the issue by running cabal haddock all instead of cabal haddock all --haddock-all. (It's a bit weird that those commands don't do the same thing, but eh.)

@phadej
Copy link
Collaborator

phadej commented Aug 22, 2022

Resolved in #613

@phadej phadej closed this as completed Aug 22, 2022
@RyanGlScott
Copy link
Contributor

Thanks, @phadej!

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

No branches or pull requests

4 participants