-
Notifications
You must be signed in to change notification settings - Fork 520
Add buildProperties to project service configuration #383
Conversation
…n as global property to setup msbuild project
… non-nullable type.
Instead of build args it should be a dictionary of properties. |
Ok, I will implement this with a dictionary. But there are entries in the dictionary that need to be transtaled as build command arguments. In this case, I extracted the "dictionary" only for the supported build configuration from the build args. A question: What to do with the all properties in the dictionary? I know what to do with the build Configuration. But the others? |
They can all be translated via /p:{Key}={Value}, it's just msbuild properties. You can choose to recognize some as first class but I wouldn't at the moment. |
…. Consider declaring the property as nullable.
I tried something different for the moment, added properties but only build Configuration as first class |
Also translated extra properties as /p:{Key}={Value} |
This change needs a test, my guess is that this will fail with --docker as well. |
…rty>) of ConfigService property to load build properties
Will check this one. |
Added support for --docker option, working on the test |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good. A few nits.
{ | ||
UseMultiphaseDockerfile = false, | ||
}; | ||
project.ContainerInfo = new ContainerInfo() { UseMultiphaseDockerfile = false, }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reformatting changes make it really hard to tell what actually changed. In the future when you send more PRs (and please send more PRs these are great) I'd refrain from doing these changes as they are a bit distracting in the diff.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before commit I'm running 'dotnet format -w.'. I don't remember changing that line explicitly. I'll be more careful next time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem. Your contributing some great stuff! Keep it coming!
* Add buildArgs to service configuration and capture build configuration as global property to setup msbuild project * Fix format * Fix - Error CS8600: Converting null literal or possible null value to non-nullable type. * Improve configuration format for build arguments * Fix whitespace format * Fix error CS8618: Non-nullable property 'Properties' is uninitialized. Consider declaring the property as nullable. * Fix error CS8601: Possible null reference assignment. * Translate non first class properties as /p:{Key}={Value} into the build command * Fix property translation * All properties are used to create the msbuild project * Change the name (to BuildProperties) and the type (to List<BuildProperty>) of ConfigService property to load build properties * Add support of build properties when tye run with --docker option * Fix ComprehensionalTest * Add tests to verify the output directory for the corresponding build configuration * Fix whitespace format * Override the correct CreateTestCasesForTheory to fix error CS0618 * Remove the usage of BuildPropertiesToOptionsMap and fix the code format
* Add buildArgs to service configuration and capture build configuration as global property to setup msbuild project * Fix format * Fix - Error CS8600: Converting null literal or possible null value to non-nullable type. * Improve configuration format for build arguments * Fix whitespace format * Fix error CS8618: Non-nullable property 'Properties' is uninitialized. Consider declaring the property as nullable. * Fix error CS8601: Possible null reference assignment. * Translate non first class properties as /p:{Key}={Value} into the build command * Fix property translation * All properties are used to create the msbuild project * Change the name (to BuildProperties) and the type (to List<BuildProperty>) of ConfigService property to load build properties * Add support of build properties when tye run with --docker option * Fix ComprehensionalTest * Add tests to verify the output directory for the corresponding build configuration * Fix whitespace format * Override the correct CreateTestCasesForTheory to fix error CS0618 * Remove the usage of BuildPropertiesToOptionsMap and fix the code format
Add buildProperties to project service configuration.
The service configuration could look like this: