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

Action/Server: rewrite showFroms to be clearer #392

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

Profpatsch
Copy link
Contributor

(This contains some of the refactor commits from #391, only the last commit changes showFroms substantially)

This pulls apart the display logic and the data munging in showFroms, for ease of understanding.
The resulting code is a bit longer, but uses a nice to understand pipeline of sortOn, groupOn and mapMaybe.

The main complications came from the fact that both the packageName and the targetModule are optional fields, but now it should be fairly straight forward.

@Profpatsch
Copy link
Contributor Author

cc @smatting

The `local` and `haddock` booleans are only used for determining the
URLs to generate, so let’s make that clear.
The function does not really deduplicate the elements, it takes and
groups. :)
Upstream does not think shadowing should be avoided, so let’s undo the
changes to shadowing.
Same with incomplete pattern warnings, we’ll just let it crash for
now.

Also drop the `Is*` prefix for the URL constructors.
CI unfortunately uses -Werror and tests against more modern versions
of GHC, so any new errors will only appear on CI.
The way the template was filled was rather hard to follow.

This tries to remedy it by splitting the code which infers the data
from the list of targets, and the code which generates the HTML.

The logic should be exactly the same, but we use a sort->groupBy to
stable-sort Targets into their packages.
@Profpatsch Profpatsch force-pushed the action-server-showFroms-rewrite branch from b6aa989 to e65973c Compare March 19, 2023 16:29
@Profpatsch
Copy link
Contributor Author

Once #391 is merged, this is only one commit.

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.

1 participant