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

Fake 5 InnoSetup Module #1890

Merged
merged 2 commits into from
Apr 29, 2018
Merged

Fake 5 InnoSetup Module #1890

merged 2 commits into from
Apr 29, 2018

Conversation

kblohm
Copy link
Contributor

@kblohm kblohm commented Apr 26, 2018

Hi,
i created a module to support Inno Setup, a tool to build installers for windows.
Since there are other installer tools not yet ported i put the module under Fake.Installer.
If u want to use that namespace for an actual Fake-Installer, i can also put it under Fake.InstallerTools or something similair.

I also have a few questions:

  • should new modules still be added into FakeLib.fsproj?
  • if not, the build script probably needs to be modified. GenerateDocs docs only looks into the "./build" folder and as far as i can see FAKE.sln is only used to generate nuget packages. At least i suppose thats why i do not get any documentation generated :)

Thanks!

@matthid
Copy link
Member

matthid commented Apr 26, 2018

Thanks! I think the namespace is nice. I don't expect a critical conflict even if we do some kind of installer as this will be in a separate InnoSetup module (or something like that)

should new modules still be added into FakeLib.fsproj?

not necessarily, the general guideline is that all new stuff needs to be available as module. If people need stuff in the old fakelib adding it is fine for me.

if not, the build script probably needs to be modified. GenerateDocs docs only looks into the "./build" folder and as far as i can see FAKE.sln is only used to generate nuget packages. At least i suppose thats why i do not get any documentation generated :)

I think you found a huge pain point ;).
Actually currently we only generate docs for "fakelib" as it contains all the new apis as well (for migration purposes). If we now have a new module outside of fakelib we probably need to tackle that issue ...

@matthid matthid changed the base branch from master to rc_9 April 26, 2018 21:27
@kblohm
Copy link
Contributor Author

kblohm commented Apr 27, 2018

Hi,
i will try to fix the documentation issue for new modules. But that should probably be another PR.
If there is anything else i can do here, just tell me.

@matthid
Copy link
Member

matthid commented Apr 27, 2018

Hi, yes I think this is ready (I actually already changed to rc_9 in order to release this) you just slightly missed the release window ;)

<li>
<a href="@(prefix)apidocs/index.html#Fake.Installer">Installer</a>
<ul>
<li><a href="@(prefix)apidocs/fake-installer-innosetup.html">InnoSetup</a></li>
Copy link
Member

Choose a reason for hiding this comment

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

This will afaik be a dead link, but yeah

@kblohm
Copy link
Contributor Author

kblohm commented Apr 27, 2018

The docs are working locally for me now.
The only problem i have is that i need to change Fake.DotNet.FSFormatting for it to work. That might be fine, but since the build-script pulls that from nuget i am not sure what to do. I can reference the .fs file in the buildscript for now. Any thoughts? Should i just create a PR and u have a look at it there?

@matthid
Copy link
Member

matthid commented Apr 27, 2018

@kblohm Yes that is quite an unfortunate bootstrapping problem, I can tackle that if needed. Usually I just #load the required .fs files temporarily in that situation.

@matthid matthid changed the title WIP: Fake 5 InnoSetup Module Fake 5 InnoSetup Module Apr 27, 2018
@matthid
Copy link
Member

matthid commented Apr 29, 2018

Thanks!

@matthid matthid merged commit b7efb9f into fsprojects:rc_9 Apr 29, 2018
@kblohm kblohm deleted the innoSetup branch April 29, 2018 11:14
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