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

haddock-project fixes #8919

Merged
merged 3 commits into from
Jul 5, 2023
Merged

haddock-project fixes #8919

merged 3 commits into from
Jul 5, 2023

Conversation

coot
Copy link
Collaborator

@coot coot commented Apr 25, 2023

This PR includes three commits which later haddock-project command:

QA Notes:

cabal haddock-project

Running cabal haddock-project should create documentation folder ./haddocks which contains:

  • ./haddocks/<package-name> folder for every dependency of the project; in particular ./haddocks/<package-name>/<package-name>.haddock should be present;
  • ./haddocks/index.html;
  • ./haddocks/quick-jump.min.js

When serving the ./haddocks folder with http server the index page should list all the local packages. Using quick jump from the index page should allow to find any identifier found in any of the local packages, when viewing documentation of one of them quick jump should allow to jump to any identifier of that package. All entities should be linked, e.g. if you find IO in the documentation, it should lead you to base documentation of IO.

cabal haddock-project --hackage

Running cabal haddock-project --hackage should create documentation folder ./haddocks which only contains documentation of local packages. Identifiers from dependencies should be linked to hackage.


Please include the following checklist in your PR:

Bonus points for added automated tests!

@coot coot requested a review from Kleidukos April 25, 2023 20:37
@coot coot marked this pull request as ready for review April 25, 2023 20:42
@coot coot force-pushed the coot/haddock-package-fixes branch from 4ad4ab9 to 5e24585 Compare April 26, 2023 07:00
@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

@Kleidukos do you know a package published on Hackage which is using visible sublibraries? I haven't found any among recent uploads.

@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

haskell/hackage-server#1119 is still open...

@Mikolaj
Copy link
Member

Mikolaj commented Apr 26, 2023

The CI failure seem to be expected at this point?

I've tentatively marked it for backport to 3.10 branch, since it fixes (semi-) regressions and it's not a huge risky PR.

@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

The CI failure seem to be expected at this point?

It's unexpected. I'll try to reproduce it locally.

@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

Looking closer, these CI failures are expected. I changed the log output of the cabal haddock command. It no longer reports that haddock created index.html file if it didn't received --gen-index file.

--gen-index result
present log location of the index.html file created
not present log directory location of the documentation

@coot coot force-pushed the coot/haddock-package-fixes branch from 5e24585 to 076f70c Compare April 26, 2023 18:07
@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

@Mikolaj do you want me to create a PR against the 3.10 branch?

@Mikolaj
Copy link
Member

Mikolaj commented Apr 26, 2023

@Mikolaj do you want me to create a PR against the 3.10 branch?

No, thank you. Mergify will do this automatically for us and I sincerely hope there won't be any conflicts.

@coot
Copy link
Collaborator Author

coot commented Apr 26, 2023

nice :), there shouldn't be any.

@coot coot force-pushed the coot/haddock-package-fixes branch 3 times, most recently from dbebb68 to 53cc424 Compare April 26, 2023 20:41
@coot coot force-pushed the coot/haddock-package-fixes branch 2 times, most recently from e0b461a to ec1027a Compare April 29, 2023 09:14
@Kleidukos
Copy link
Member

@Kleidukos do you know a package published on Hackage which is using visible sublibraries? I haven't found any among recent uploads.

@coot I don't even know if it's allowed. Hackage doesn't seem to support it?

@ulysses4ever
Copy link
Collaborator

haskell/hackage-server#1119 is still open...

It is but a comment there
haskell/hackage-server#1119 (comment) says it should work. Just no one tried it...

@coot coot force-pushed the coot/haddock-package-fixes branch from ec1027a to 4d2e7bd Compare May 13, 2023 20:09
Copy link
Member

@Kleidukos Kleidukos left a comment

Choose a reason for hiding this comment

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

Good for me!

@coot
Copy link
Collaborator Author

coot commented May 20, 2023

I pushed one more commit to fix #8958.

@ulysses4ever
Copy link
Collaborator

@coot could you add a test that ensures that dependencies are handled correctly?

@coot
Copy link
Collaborator Author

coot commented May 20, 2023

yes, I am planning to do that.

@coot coot force-pushed the coot/haddock-package-fixes branch from ce698df to 24c4c07 Compare May 21, 2023 13:15
@ulysses4ever ulysses4ever removed their request for review June 29, 2023 06:15
Copy link
Member

@Mikolaj Mikolaj left a comment

Choose a reason for hiding this comment

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

After a quick look, LGTM, but a more thorough review would be welcome.

Whether we backport is up to @Kleidukos.

@Kleidukos
Copy link
Member

@Mergifyio backport 3.10

@mergify
Copy link
Contributor

mergify bot commented Jun 29, 2023

backport 3.10

✅ Backports have been created

@Kleidukos
Copy link
Member

@julialongtin would you like to perform the QA procedure on this PR? :)

@ulysses4ever
Copy link
Collaborator

@coot you reached 2 approvals now, so you can put the merge-me label when you feel like it and hand it over to the bot (it will wait for 2 days before merge as usual). Thank you for your contribution and also for addressing my comments in particular!

@coot coot added the merge me Tell Mergify Bot to merge label Jul 2, 2023
@coot
Copy link
Collaborator Author

coot commented Jul 2, 2023

Thanks @ulysses4ever, @Kleidukos & @Mikolaj for cooperation.

@Mikolaj
Copy link
Member

Mikolaj commented Jul 3, 2023

Thank you, @coot. That's a long struggle, but many people waited years for these features and here they are finally, more and more usable.

@coot
Copy link
Collaborator Author

coot commented Jul 5, 2023

Shouldn't mergify do its job by now?

@ulysses4ever
Copy link
Collaborator

ulysses4ever commented Jul 5, 2023

I think you commented before the full 48 hours passed. And now the timer is probably reset. But let's fix it.

@ulysses4ever ulysses4ever added merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days and removed attention: needs-review labels Jul 5, 2023
@Mikolaj
Copy link
Member

Mikolaj commented Jul 5, 2023

@Mergifyio refresh

@mergify
Copy link
Contributor

mergify bot commented Jul 5, 2023

refresh

✅ Pull request refreshed

@Mikolaj
Copy link
Member

Mikolaj commented Jul 5, 2023

It says "Unable to update: user coot is unknown. Please make sure coot has logged in Mergify dashboard."

What was that about?

@ulysses4ever
Copy link
Collaborator

In the failure like this (" ❌ Queue: Embarked in merge train"), rebase is usually the way to go (Mergify fails to rebase a PR from a non-admin user on its own). "Refresh" works better when the bot thinks that some checks are not satisfied.

@ulysses4ever
Copy link
Collaborator

@mergify rebase

coot added 3 commits July 5, 2023 18:43
This patch fixes a bug where a sublibrary overwrites a library
directory.  This leads to overwriting `*.haddock` files, which results
in missing entries in various indexes (e.g. the html index,
quicksearch).
This patch makes `haddock-project` use `--local` option by default.
Since its the default, it is removed. Also `--gen-index`,
`--gen-contents`, `--hyperlinked-source` and `--quickjump` are removed
since they are always turned on.

Added a haddock-project test.
`haddock-project` command need to build the project so that haddocks of
the dependencies are available in the store.

Fixes #8958.
@mergify
Copy link
Contributor

mergify bot commented Jul 5, 2023

rebase

✅ Branch has been successfully rebased

@mergify mergify bot merged commit a93d693 into master Jul 5, 2023
@mergify mergify bot deleted the coot/haddock-package-fixes branch July 5, 2023 21:07
@ulysses4ever ulysses4ever mentioned this pull request Jul 17, 2023
5 tasks
This was referenced Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cabal-install: cmd/haddock merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge me Tell Mergify Bot to merge
Projects
None yet
4 participants