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

Error message was not direct enough. #4183

Merged
merged 2 commits into from
Oct 3, 2018
Merged

Error message was not direct enough. #4183

merged 2 commits into from
Oct 3, 2018

Conversation

voronoipotato
Copy link
Contributor

Scott Nimrod, otherwise completely capable F# developer was flummoxed by this in a live stream. Added clear resolution steps to error message. It's a rather roundabout way to say "You've got an extra space there".

https://youtu.be/_6jV1n0fL00?t=10m25s

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

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

I like the change. I have one suggestion.

@@ -1041,8 +1041,8 @@ lexHashBangMustBeFirstInFile,"#! may only appear as the first line at the start
1186,parsInvalidProperty,"Invalid property getter or setter"
1187,parsIndexerPropertyRequiresAtLeastOneArgument,"An indexer property must be given at least one argument"
1188,tastInvalidAddressOfMutableAcrossAssemblyBoundary,"This operation accesses a mutable top-level value defined in another assembly in an unsupported way. The value cannot be accessed through its address. Consider copying the expression to a mutable local, e.g. 'let mutable x = ...', and if necessary assigning the value back after the completion of the operation"
1189,parsNonAdjacentTypars,"Type parameters must be placed directly adjacent to the type name, e.g. \"type C<'T>\", not type \"C <'T>\""
1190,parsNonAdjacentTyargs,"Type arguments must be placed directly adjacent to the type name, e.g. \"C<'T>\", not \"C <'T>\""
1189,parsNonAdjacentTypars,"Remove spaces after type, e.g. \"type C<'T>\", not type \"C <'T>\" type parameters must be placed directly adjacent to the type name"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change:

Remove spaces between the type name and type parameter, e.g. \"type C<'T>\", not type \"C   <'T>\". Type parameters must be placed directly adjacent to the type name

This way it's a bit more specific about what the issue is. It also fixes a run-on sentence.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes that sounds better. Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

Like a twerp I looked at the conversation, not the files :-(

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@KevinRansom what does that mean? Is something wrong?

Copy link
Member

Choose a reason for hiding this comment

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

@voronoipotato no, I nothing is wrong.

@KevinRansom
Copy link
Member

@dotnet-bot test this please

@cartermp
Copy link
Contributor

@dotnet-bot test this please

@voronoipotato would you like to make the suggested change?

@voronoipotato
Copy link
Contributor Author

sure I can do that.

@voronoipotato
Copy link
Contributor Author

sorry about the bumpy ride here, first time doing a pull request on a project with a significant number of contributors.

@cartermp
Copy link
Contributor

No worries!

@cartermp
Copy link
Contributor

@brettfo Any idea about what this error in CI is about?

D:\j\w\release_ci_pa---3f142ccc\Packages\XliffTasks.0.2.0-beta-000076\build\XliffTasks.targets(90,5): error : '..\xlf\FSComp.txt.en.xlf' is out-of-date with 'obj\net40\FSComp.resx'. Run msbuild /t:UpdateXlf to update .xlf files or set UpdateXlfOnBuild=true to update them on every build, but note that it is strongly discouraged to set UpdateXlfOnBuild=true in official/CI build environments as they should not modify source code during the build. [D:\j\w\release_ci_pa---3f142ccc\src\fsharp\FSharp.Compiler.Private\FSharp.Compiler.Private.fsproj]

@cartermp
Copy link
Contributor

@dotnet-bot test this please

@KevinRansom
Copy link
Member

@cartermp phillip ... that is a notification that the xlif files are not uptodate. You need to ensure they are upto date. The cli machine does not edit PRs

use these commands to update the pr:

cd $(yourroot)\src\fsharp\FSharp.Compiler.Private
msbuild /t:UpdateXlf

@KevinRansom KevinRansom closed this Oct 2, 2018
@KevinRansom KevinRansom reopened this Oct 2, 2018
@voronoipotato
Copy link
Contributor Author

Oh beans I deleted my repository. Is this a problem?

@KevinRansom KevinRansom merged commit ad11b5a into dotnet:master Oct 3, 2018
@KevinRansom
Copy link
Member

@voronoipotato no probs mate, apparently git hub is smart enough to cope with that kind of thing.

Kevin

@cartermp cartermp added this to the 16.0 milestone Nov 21, 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.

None yet

3 participants