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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Visual Basic support to AssemblyFileInfo task and make Namespace optional in config #471

Merged
merged 2 commits into from Jun 16, 2014

Conversation

@petersondrew
Copy link
Contributor

commented Jun 13, 2014

This pull request contains one new feature and one breaking change.

First, as I'm sure many of us have VB code in the wild, I've added support for generating AssemblyInfo.vb files for Visual Basic projects. I was unsure whether it was better to call the function CreateVisualBasicAssemblyInfoWithConfig or CreateVBAssemblyInfoWithConfig, so I played it safe and went with the former. I've not added any tests specific to generating the VB file as there were no architectural changes to the attributes, and the existing tests did not cover the differences between the AssemblyInfo file layout between languages.

Second, I've added a breaking change to the AssemblyInfoFileConfig record in that it is now a class. This was done to allow the Namespace field to be optional for code passing an AssemblyInfoFileConfig to the CreateXXAssemblyInfoWithConfig functions, as it's pointless to write a namespace declaration to the AssemblyInfo file if no class is being generated, and it's confusing to require the developer to specify the namespace when they want to specify that no class should be generated to begin with. We could alternatively turn the defaults on their head such that no class is generated by default, but I feel that would silently change the behavior of people's build scripts, where this would only effect those that wish to disable class generation anyway.

Third, I'm still a novice when it comes to F#, so if I've made any poor stylistic choices in my coding please let me know 馃槈.

Optional Namespace in AssemblyInfoFileConfig
Namespace should be optional as it's not required if GenerateClass = false.
If GenerateClass is false, a namespace should not be added to the
AssemblyInfo file.

- Make AssemblyInfoFileConfig a class with optional UseNamespace
- Alter CreateCSharpAssemblyInfoWithConfig and
  CreateVisualBasicAssemblyInfoWithConfig to not write a namespace line
  unless a class is being generated
@forki

This comment has been minimized.

Copy link
Member

commented Jun 16, 2014

Thanks for adding the VB support. I'll need to check if this breaks octokit, but from what I can say it looks good.

forki added a commit that referenced this pull request Jun 16, 2014
Merge pull request #471 from petersondrew/master
Add Visual Basic support to AssemblyFileInfo task and make Namespace optional in config

@forki forki merged commit 2175b62 into fsharp:master Jun 16, 2014

2 checks passed

continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci The Travis CI build passed
Details
forki added a commit that referenced this pull request Jul 8, 2014
BREAKING CHANGE: API for CreateAssemblyInfoWithConfig was set back to鈥
鈥 original version

This resets the breacking change introduced in #471
@forki

This comment has been minimized.

Copy link
Member

commented Jul 8, 2014

Unfortunately it had a breaking change. I didn't see that before.

@petersondrew

This comment has been minimized.

Copy link
Contributor Author

commented Jul 8, 2014

Yes, sorry about that. I noted it in the pull request; I should have verified that you had seen that.

Did this break things for anyone?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.