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

Internal refactorings to install command. #6746

Conversation

m-renaud
Copy link
Collaborator

@m-renaud m-renaud commented Apr 30, 2020

Pulls out implementation details into standalone functions. This makes the required inputs clear and removes from-afar data dependencies.


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 (add file to changelog.d directory).
  • The documentation has been updated, if necessary.

Please also shortly describe how you tested your change. Bonus points for added tests!

@m-renaud m-renaud marked this pull request as draft April 30, 2020 21:19
@m-renaud m-renaud mentioned this pull request Apr 30, 2020
@@ -628,6 +464,164 @@ installAction ( configFlags, configExFlags, installFlags
haddockFlags testFlags benchmarkFlags
globalConfigFlag = projectConfigConfigFile (projectConfigShared cliConfig)


getClientInstallFlags :: Verbosity -> GlobalFlags -> ClientInstallFlags -> IO ClientInstallFlags
Copy link
Collaborator

Choose a reason for hiding this comment

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

The name should hint that this is used for ignore-project cases.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I'm not familiar with ignore-project cases or what they imply so can't really come up with a better name, let me know if you have a suggestion.

cabal-install/Distribution/Client/CmdInstall.hs Outdated Show resolved Hide resolved
cabal-install/Distribution/Client/CmdInstall.hs Outdated Show resolved Hide resolved
@m-renaud m-renaud requested a review from phadej May 2, 2020 02:50
@m-renaud m-renaud changed the title WIP: Internal refactorings to install command. Internal refactorings to install command. May 2, 2020
@m-renaud m-renaud marked this pull request as ready for review May 2, 2020 02:51
@phadej phadej mentioned this pull request May 6, 2020
@phadej
Copy link
Collaborator

phadej commented May 7, 2020

Please squash into single commit and merge once CI is green

@m-renaud m-renaud force-pushed the m-renaud-cmd-install-refactor-installCommand branch from be2a352 to d080622 Compare May 7, 2020 21:45
This simply extract logic out of installAction into separate helper
functions. This makes the inputs to each of these explicit and removes data
dependencies from a distance. This will make it easier in the future to pull out
behaviour to be shared between the 'install' command and the 'env' command.
@m-renaud m-renaud force-pushed the m-renaud-cmd-install-refactor-installCommand branch from d080622 to 95d4917 Compare May 8, 2020 02:38
@m-renaud m-renaud merged commit 9714c53 into haskell:master May 8, 2020
@phadej phadej mentioned this pull request Jul 10, 2020
@phadej phadej added this to the 3.4.0.0-rc1 milestone Jul 13, 2020
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.

2 participants