Pack localized assemblies. by JohanLarsson · Pull Request #1816 · fsprojects/Paket

JohanLarsson · 2016-07-17T12:11:04Z

Second attempt at #1806.

Should fix Paket 3.7.0.0 broke paket pack #1810.
Printing warning instead of throwing if satellite dll is not found. Added integration test for this.
Better pattern for finding languages.
Checking that matches for the regex is a culture name.

Thank you for all the help @smoothdeveloper!

smoothdeveloper · 2016-07-17T12:26:05Z

+    paket ("pack -v output \"" + outPath) scenario |> ignore
+    ZipFile.ExtractToDirectory(package, outPath)
+
+    Path.Combine(outPath, "lib", "net45", "LocalizedLib.dll") |> checkFileExists


Could you use </> operator defined in Paket.IO (it is AutoOpen so you don't even have to open the namespace):

outPath </> "lib" </> "net45" </> "LocalizedLib.dll" |> checkFileExists

While it looks sweet I have not seen it in the rest of the code so not sure. Trying to stay consistent with the codebase. Also as a noob Path.Combine(...) is nice as I know what it does.
As always, thanks for taking the time to read!

Yes, it's minor thing.

That same operator is defined in FAKE and pretty much any codebase dealing with paths so I thin it is not too far from reach for average F# developer.

You can use go to definition on operators also 😄

forki · 2016-07-18T06:26:45Z

Why do we show warnings and when? I mean having no satellite assemblies is the 95% case

JohanLarsson · 2016-07-18T08:06:50Z

The warning is shown here, it happens when:

A localization for language is found in the project file.
No matching ../{language}/foo.resources.dll is found.
In the previous version I threw here but it was too harsh I think.
I can remove it of course but think it can be a good reminder to build before running paket pack.

No localization should not give a warning.

Fixes since previous attempt: - fsprojects#1810 should be ok now. - Outputting warning instead of throwing if satellite dll is not found. - Better pattern for finding languages. - Checking that matches for the regex is a culture name.

smoothdeveloper · 2016-07-18T12:25:29Z

+open System.Collections.Generic
+open System.Globalization
+
+[<CompilationRepresentation (CompilationRepresentationFlags.ModuleSuffix)>]


Unless you have a type Cultures under Paket namespace, I don't think we need those attributes (which is used to disambiguate between module and type bearing same name).

I copy pasted them from the Cache module, no idea what they mean.

This is needed for Cache module because there is also a Cache type under same namespace.

F# will coallesce both to expose them as Cache but the module need to be given that attribute which under the hood suffixes the static class with Module.

We don't need it for this Cultures since there is no Cultures type that would conflict.

Thanks for the explanation, removing it.
Oh, too late.

there is always room for new PR ;-)

I was gonna try to fix the uri issue but was too slow :D

That one is a bit tricky. not sure if my fix was good. have to think about it a bit more.

forki · 2016-07-18T15:14:20Z

there is still one related test failing on mono: https://travis-ci.org/fsprojects/Paket/builds/145555619

JohanLarsson · 2016-07-18T15:43:38Z

Checking it out, didn't see that one before, strange.

forki · 2016-07-18T16:10:28Z

I assume there is something not commited

forki · 2016-07-18T16:15:13Z

#1816 pack localized happy path fails on my machine with:

System.Exception : Paket failed with:
Could not find 'paket.dependencies'. To use Paket with this solution, please run 'paket init' first.
If you have already run 'paket.init' then ensure that 'paket.dependencies' is located in the top level directory of your repository.
Like this:
MySourceDir
  .paket
  paket.dependencies

JohanLarsson · 2016-07-18T16:17:06Z

Looks like the templates are missing, must have dumbed it during some rebase. Opening a new WIP PR for fixing it.

smoothdeveloper reviewed Jul 17, 2016
View reviewed changes

JohanLarsson force-pushed the pack_localized branch 3 times, most recently from f45ed83 to 2c47583 Compare July 18, 2016 00:32

JohanLarsson changed the title ~~Pack localized assemblies. WIP~~ Pack localized assemblies. Jul 18, 2016

Pack localized assemblies take II.

f9508c6

Fixes since previous attempt: - fsprojects#1810 should be ok now. - Outputting warning instead of throwing if satellite dll is not found. - Better pattern for finding languages. - Checking that matches for the regex is a culture name.

JohanLarsson force-pushed the pack_localized branch from 2c47583 to f9508c6 Compare July 18, 2016 08:21

smoothdeveloper reviewed Jul 18, 2016
View reviewed changes

forki merged commit f9508c6 into fsprojects:master Jul 18, 2016

Conversation

JohanLarsson commented Jul 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanLarsson Jul 17, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

forki commented Jul 18, 2016

Uh oh!

JohanLarsson commented Jul 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JohanLarsson Jul 18, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

forki commented Jul 18, 2016

Uh oh!

JohanLarsson commented Jul 18, 2016

Uh oh!

forki commented Jul 18, 2016

Uh oh!

forki commented Jul 18, 2016

Uh oh!

JohanLarsson commented Jul 18, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JohanLarsson commented Jul 17, 2016 •

edited

Loading

JohanLarsson Jul 17, 2016 •

edited

Loading

JohanLarsson commented Jul 18, 2016 •

edited

Loading

JohanLarsson Jul 18, 2016 •

edited

Loading