Minor cleanup for Build Task - #3969
Conversation
KevinRansom
commented
Nov 17, 2017
KevinRansom
commented
- Remove .fsi files
- re-align some methods, trim some redundant spaces
- Always build in runningonmono rather that using #if
- We don't support Itanium .... no really we don't
… just like on desktop
| | "ANYCPU", _, _ -> "anycpu" | ||
| | "X86" , _, _ -> "x86" | ||
| | "X64" , _, _ -> "x64" | ||
| | "ITANIUM", _, _ -> "Itanium" |
|
@dotnet-bot test Windows_NT Release_ci_part1 Build please |
| /// Return a list of the arguments (with no quoting for the cmd.exe shell) | ||
| member x.CapturedArguments() = | ||
| List.rev args | ||
| member x.CapturedArguments() = List.rev args |
There was a problem hiding this comment.
Possible additional cleanup: Whenever you see a mutable F# list plus a call to List.rev it's almost always cleaner to make it a use of a ResizeArray (i.e. System.Collections.Generic.List<T>) without the need for a reverse. It's a common F# code smell
| type [<Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly")>] Fsc() as this = | ||
| inherit ToolTask() | ||
| type | ||
| [<Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Naming", "CA1704:IdentifiersShouldBeSpelledCorrectly")>] |
There was a problem hiding this comment.
You can place the attribute before the type keyword, it looks much nicer,
There was a problem hiding this comment.
well that's the whole point of cleanup ... consider it done :-)
| do | ||
| let runningOnMono = try System.Type.GetType("Mono.Runtime") <> null with e -> false | ||
| if not runningOnMono then | ||
| typeof<ToolTask>.InvokeMember("YieldDuringToolExecution", |
There was a problem hiding this comment.
Mono probably has this available these days in the DLL - if so do this.YieldDuringToolExecution <- true is much cleaner and long-term safer.
If not I think I prefer the original code using #if
There was a problem hiding this comment.
@dsyme xbuild is deprecated in favour of msbuild. So yes I will remove the running on mono check.
| // ===================================================================================================== | ||
| // Todo: x.Compile(compile:System.Converter<int,int>, flags:string[], sources:string[]) for VS2017.7 | ||
| // Earlier buildtasks usesd System.Converter<int,int> for cross platform we are moving to Func<int> | ||
| // This is so that during the interim, earlier VS's will still load the OSS project |
There was a problem hiding this comment.
This is so that during the interim, earlier VS's will still load the OSS project
Thanks for looking after this case
4 participants
