-
Notifications
You must be signed in to change notification settings - Fork 0
Ss config n2 #19
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
Open
AdamBJ
wants to merge
26
commits into
master
Choose a base branch
from
ss-config-n2
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Ss config n2 #19
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit starts working to try and make the Umple plugin behave similarly to the Scala and Antlr plugins. I've realized that having the configuration in the `sourceSets` is not worth-while, however, we can use them to add extra configuration (mostly dependency checks). The current state has a global configuration closure (`umple`) that specifies the information on how to generate things _per project_. The idea would be to allow for the specification of an array of generators and we always place them into a subdirectory that is `build/generated-src/$lang`. Additionally, I've added some testing that does not require the `integrationTests` style testing, that should simplify some testing -- we still need the integration ones, though. [skip ci]
[skip-CI] This commit contains a "proof of concept" that shows how we can store the `UmpleGenerateTask` object as a member variable of the `DefaultUmpleSourceSet`. By maintaining an `UmpleGenerateTask` member variable, we can update the configuration variables stored in `UmpleGenerateTask` once user-defined overrides have been read into the `DefaultUmpleSourceSet`. This commit also renames any `sourceSet` variables to something else (bb2 and abc123 in `UmpleGradlePlugin.groovy`) @Nava2, to get this working on your machine, make sure your umple files are located at src/" + sourceSet.getName() + "/umple in your project directory. You should set the `umpleFilePath` in the `umple` closure of the project's `build.gradle` file to point to `master.ump` in the same location. Here's an example of how to configure the sourceSets in your project's `build.umple` file: sourceSets { main { umple{ umpleFilePath = file('path/to/your/umple/folder') } } } I'm not sure if the path has to be absolute or not. If relative doesn't work, use absolute for now. Let me know if you have any problems getting things running (you should be able to run things and only get the "No value has been specified for property 'sourceSet'. error").
Tried to address some of the concerns from #14, but still needs integration testing. @AdamBJ solved most of the leg work, I fought with it a bit more and figured out how to actually get the information back from the sourceSet closures. By adding the properties to the `UmpleSourceSet` (via the `UmpleOptions` interface), we can now set the properties that we want for running Umple. This is because when the `Closure` is applied, the `delegate` used for figuring out what fields are what is set to be the `SourceSet` that it is called against (in this case, `UmpleSourceSet`). See [1] for more information on how to apply `Closure`s. The TODO from all of this: 1. Create a better testing suite -- the "properties-as-strings" approach is nasty, we need to do better 2. Add more integration testing 3. Verify what I did makes sense.. I think I may have excessively created the "config-style" as I'm not sure how many tasks are made per project [1] http://groovy-lang.org/closures.html#closure-this
This commit connects a lot of plugin components that weren't communicating properly before. Here's the summary of what I did (see the in-code comments for more information):
Changed the way the UmpleGenerateTask.groovy sets configuration information. Here's what happens now:
The default umple closure (i.e. the umple {} that's declared by the user outside of the sourceSets closure) is created in UmpleGradlePlugin.groovy at line 39. The SourceSet specific version of the umple closure is created for each source set at line 59. If the user has specified custom configuration information it is stored in one of these closures.
Before we go run the UmpleGenerateTask, we first call setUmpleGenerateConfig (formerly loadGlobals) at line 69 of UmpleGenerateTask.groovy. This function checks all the locations where user configuration information could be stored and makes that information available to the UmpleGenerateTask before the task is run. SourceSet specific configuration that the user specifies takes priority over default configuration they specify. If the user hasn't provided SourceSet specific or default configuration information for a particular property we get the configuration information from the hard-coded values in UmpleOptions.groovy.
There are a bunch of other little tweaks included in this commit too, but the configuration change is the big one. You can have a look at the TODO tags scattered throughout the source to see what still needs to be done. I'm going to add some comments on GitHub for the issues I think are more important to resolve.
[skip ci]
We need to add more tests, but I think the plugin is at the point that it can be used in actual projects. I've added the start of a "replacement" to the integrationTests to use normal JUnit instead. TODO * Add more tests for both unit and integration style testing * Clean up more edge cases * Add extra parameters * Verify java integration is better :)
- Fixed the stack overflow error - minor bug fixes - Re-enabled all tests, they're green locally. Travis will still fail, though, because it will download an old version of the plugin jar from the plugin portal. Once we publish the changes we've made Travis should be green [skip ci]
- changed `dependsFlag` to `compileGenerated` - changed default value for `compileGenerated` to true - bug fixes
- Updated readme to reflect new plugin interface - Added a "Base project" that users can study / use as a template. I've not included the Guava dependency in the hopes that we'll soon resolve that.
Member
Contributor
|
@vahdat-ab yeah, I think we just use the |
- Use can now optionally specify a path relative to the project directory for the `master` configuration value by setting the `customMasterFlag` to true - Refactored UmpleGradlePlugin.groovy so that it's not just one big method - Fixed a bug caused by incorrectly setting the `srcDir` property for the `SourceDirectorySet` we use to track the Umple files
…of testing. Other minor changes - New style follows best practices for Property (no direct calls to put and get) - Gets rid of a good number of hard coded strings - Less calls to toString(), though we're still using strings in the assertEquals stuff - Enabled printing to system out for the `test` task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR moves the plugin away from the
ext-style configuration to the antlr/scala-style configuration.Tagging @vahdat-ab so you can see what we've been up to. No need to fully review this PR though.