Skip to content

[#5503] Add filterHaddockFlags for new-haddock #5569

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

Closed
wants to merge 1 commit into from

Conversation

chshersh
Copy link
Member

@chshersh chshersh commented Sep 7, 2018


Please include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Any changes that could be relevant to users have been recorded in the changelog.
  • The documentation has been updated, if necessary.
  • If the change is docs-only, [ci skip] is used to avoid triggering the build bots.

I've tried to add additional logic suggested by the issue. But I still see the error:

Build profile: -w ghc-8.4.3 -O1
In order, the following will be built (use -v for more details):
 - network-2.7.0.2 (first run)
cabal: Unrecognised flags: lib:network
Warning: Failed to build documentation for network-2.7.0.2.

Copy link
Member

@alexbiehl alexbiehl left a comment

Choose a reason for hiding this comment

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

Great! Thanks @chshersh! And sorry for the oversight!

@hvr
Copy link
Member

hvr commented Sep 7, 2018

@alexbiehl the problem appears to be (haven't investigated enough to be sure) that build-type:configure throws the package into legacy mode and thus disables per-component -- this appears to disable support for passing arguments such as lib:network to Setup.hs haddock

@chshersh I'm afraid, we need to do a bit more here; we need to e.g. extend the arg filtering to detect this case somehow

@hvr
Copy link
Member

hvr commented Sep 7, 2018

@alexbiehl @chshersh take a look at this here

noExtraFlags a

(IOW, this was an oversight of #5226)

I'm afraid we've been looking at the wrong place to fix this...

Copy link
Member

@hvr hvr left a comment

Choose a reason for hiding this comment

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

different fix needed:

  • try using readHookWithArgs for preHaddock
  • (maybe) write a regression test-case

@chshersh maybe open a separate PR; I'd like to leave this PR open as it may still have merit on its own for consistency reasons

@chshersh
Copy link
Member Author

chshersh commented Sep 7, 2018

@hvr Sure, no problems. I will do the work in separate PR 👌

@hvr
Copy link
Member

hvr commented Sep 8, 2018

As discussed w/ @chshersh and due to the urgency involved I've created #5573 myself

@23Skidoo 23Skidoo mentioned this pull request Sep 17, 2018
@chshersh
Copy link
Member Author

@hvr @23Skidoo Could you, please, provide some updates on what is required for this PR to be done? Am I right that this is resolved in #5573 or is something needs to be done here as well?

@chshersh chshersh closed this Sep 21, 2019
@chshersh chshersh deleted the chshersh/5503-haddock branch September 21, 2019 10:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants