Skip to content

Conversation

@AdamBJ
Copy link
Contributor

@AdamBJ AdamBJ commented Apr 9, 2017

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.

Nava2 and others added 13 commits March 15, 2017 22:10
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.
@AdamBJ AdamBJ requested a review from Nava2 April 9, 2017 02:42
@vahdat-ab
Copy link
Member

A late answer :-): @Nava2 enums are not native elements in Umple. The one you might point out is about state machines such as Fruit {apple, orange}, but this is not an enumeration that can be reused easily. I described the reason in this issue. It should be fixed by the end of this semester.

@Nava2
Copy link
Contributor

Nava2 commented Apr 9, 2017

@vahdat-ab yeah, I think we just use the Class<> types for the different languages. Though, I saw the work being done on Enums which means it could be reused rather than having our own (which I think is the best option). 👍

AdamBJ added 12 commits April 9, 2017 16:51
- 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants