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.

Sign up for GitHub

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

Jump to bottom

[#333] Use key-value map for extra files #372

Merged
merged 5 commits into from
Nov 5, 2019
Merged

[#333] Use key-value map for extra files #372

merged 5 commits into from
Nov 5, 2019

Conversation

chshersh
Copy link
Contributor

@chshersh chshersh commented Nov 5, 2019 •
edited
Loading

Resolves #333

@chshersh chshersh added feature config TOML configurations labels Nov 5, 2019
@chshersh chshersh requested a review from vrom911 as a code owner November 5, 2019 07:38
@chshersh chshersh self-assigned this Nov 5, 2019
hint-man[bot]
hint-man bot approved these changes Nov 5, 2019
View reviewed changes
Copy link
Contributor

@hint-man hint-man bot left a comment

Choose a reason for hiding this comment

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

There is no place for me here... I will choose the truth I like.

vrom911
vrom911 approved these changes Nov 5, 2019
View reviewed changes
Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

This looks great all together, thanks for the huge work! 👍

The only thing left is to update the CHANGELOG, write migration note and update README with this feature accordingly. Can be done separately though 🙂

summoner-cli/src/Summoner/Config.hs
@@ -59,12 +61,13 @@ data ConfigP (p :: Phase) = Config
, cBench :: !Decision
, cPrelude :: !(Last CustomPrelude)
, cExtensions :: ![Text]
, cWarnings :: ![Text]
, cGhcOptions :: ![Text] -- ^ GHC options to add every stanza
, cWarnings :: ![Text] -- ^ DEPRECATED: TODO: remove in 1.4.
Copy link
Member

Choose a reason for hiding this comment

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

Created #374 to track this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that's very helpful! It's indeed a good idea to create issues to have a smoother deprecation period. Nice!

@chshersh
Copy link
Contributor Author

chshersh commented Nov 5, 2019

@vrom911 It's not a problem to update this PR, I will do this 🙂 Just to finish with this issue here 🙂

vrom911
vrom911 approved these changes Nov 5, 2019
View reviewed changes
Copy link
Member

@vrom911 vrom911 left a comment

Choose a reason for hiding this comment

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

Looks cool!

@vrom911 vrom911 merged commit 27efed3 into master Nov 5, 2019
@vrom911 vrom911 deleted the map branch November 5, 2019 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hint-man hint-man[bot] hint-man[bot] approved these changes

@vrom911 vrom911 vrom911 approved these changes

Assignees

@chshersh chshersh

Labels
config TOML configurations feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Use key-value map for extra files
2 participants
@chshersh @vrom911