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

Migrated UserInputHelper into Fake.Core.UserInput #1997

Merged
merged 5 commits into from
Jun 17, 2018

Conversation

nelak
Copy link
Contributor

@nelak nelak commented Jun 13, 2018

No description provided.

/// UserInput.getUserInput prompt
[<RequireQualifiedAccess>]
module UserInput =
let internal erasePreviousChar () =
Copy link
Member

Choose a reason for hiding this comment

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

The code if the module needs to be intended to the right in order to get rid of the warning:

C:\projects\fake-6w516\src\app\Fake.Core.UserInput\UserInput.fs(11,1): warning FS0058: Possible incorrect indentation: this token is offside of context started at position (10:1). Try indenting this token further or using standard formatting conventions. [C:\projects\fake-6w516\src\app\Fake.Core.UserInput\Fake.Core.UserInput.fsproj]

Basically just intend from line 11 to line 63

///
/// UserInput.getUserInput prompt
[<RequireQualifiedAccess>]
module UserInput =
Copy link
Member

Choose a reason for hiding this comment

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

This module is copied from an existing one from FakeLib, correct? Can we change the obsolete warnings of the existing module in FakeLib to point to this one?

Copy link
Member

Choose a reason for hiding this comment

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

One extension to that: We also need to add this new module to FakeLib in order to enable the migration path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should be addressed by linking the file in FakeLib which referenced by the Legacy-FAKE solution right?

@matthid
Copy link
Member

matthid commented Jun 13, 2018

The module itself looks good, thanks! Just some organizational stuff left...

@nelak
Copy link
Contributor Author

nelak commented Jun 14, 2018

I believe this should be ready now, let me know if further adjustments are needed

@matthid
Copy link
Member

matthid commented Jun 17, 2018

let me know if further adjustments are needed

The only thing that comes to mind it to add a link to the api-docs in https://github.com/fsharp/FAKE/blob/release/next/help/templates/template.cshtml
But I can do that if I'm faster with merging.

Otherwise it looks good, thanks!

@nelak
Copy link
Contributor Author

nelak commented Jun 17, 2018

Added the reference in templates.cs based on the links generated found in docs/apidocs

@matthid
Copy link
Member

matthid commented Jun 17, 2018

Yes exactly, thanks!

@matthid matthid merged commit 15b55d2 into fsprojects:release/next Jun 17, 2018
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.

2 participants