You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by geomacy <gi...@git.apache.org> on 2016/03/09 23:13:12 UTC
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
GitHub user geomacy opened a pull request:
https://github.com/apache/brooklyn-client/pull/4
Add -Pno-go-client profile
To permit skipping of build for users without Go.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/geomacy/brooklyn-client no-go-client-profile
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-client/pull/4.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #4
----
commit 6e868db0e9de64c8fe389db38df798d1586b919c
Author: Geoff Macartney <ge...@cloudsoftcorp.com>
Date: 2016-03-09T20:47:18Z
Add -Pno-go-client profile to permit skipping of build for users without Go.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on a diff in the pull request:
https://github.com/apache/brooklyn-client/pull/4#discussion_r55639789
--- Diff: pom.xml ---
@@ -50,58 +50,75 @@
<target>all</target>
</properties>
- <build>
- <plugins>
- <plugin>
- <artifactId>maven-antrun-plugin</artifactId>
- <version>1.8</version>
- <executions>
- <execution>
- <id>process-build-all</id>
- <phase>compile</phase>
+
+ <profiles>
+
+ <profile>
+ <id>no-go-client</id>
+ </profile>
+
+ <profile>
+ <id>go-client</id>
+ <activation>
+ <activeByDefault>true</activeByDefault>
--- End diff --
Will be skipped when activating any profile, not just `no-go-client`. For example when testing (`-PIntegration`). Could use a property activator instead - see [deployTo](https://github.com/apache/brooklyn-server/blob/master/parent/pom.xml#L1779) example.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194768427
thx @neykov i think you are right what i propose wouldn't work anyway, the profile modules list would be additive so making it empty would have no effect not skip the module. we need `if profile not set` which i don't yet see how to do.
maybe properties are the way to go.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194775338
we still have the problem that `activeByDefault` will not be activated if a different profile is specified explicitly, i.e. if we say `-PIntegration` we would skip the go client build!
personally i prefer something explicit in the code. poking around i think properties are definitely the way to do. will work on update (but @neykov @geomacy if you think otherwise do say!)
also `!` has to be escaped on shells which is tedious
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194759161
A wrapper module doesn't bring much to the table, makes the build more complex I think. You'd need the same workarounds elsewhere to depend on the artifact and to activate the profile for the modules.
The important thing is that we don't generate an empty zip file. Whether there's a pom file uploaded to artifactory doesn't really matter (for the client module).
Also keep in mind we'd need the same workarounds for the rpm build.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-client/pull/4
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194756901
yes i think we'd keep https://github.com/apache/brooklyn-dist/pull/14
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194752153
this is a complex way to exclude the client, and roundabout it feels as it suggests the module is built it just defines a no-op build. also with maven (as you've discovered with the `activation` logic) inverting a profile is convoluted and prone to errors.
do you think it would be better to exclude the module altogether if the profile is set?
unfortunately to do this in the uber pom it seems there is no way to avoid listing the modules twice, once with `brooklyn-client` and once without. maybe the least messy way would be to create a new pom in `brooklyn-client/pom-wrapper/` which has a default `<modules>` list pointing at the parent but if `no-go-client` profile is set the `<modules>` list would be empty. wdyt? /cc @aledsage @rdowner @neykov
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194756045
I guess something like that might be made to work but I'd have to try it out. At the same time the solution needs to include both avoiding the Go client build and also avoiding any attempt to package the artifacts if you know you didn't build it, so brooklyn-dist will also need to be aware of the profile (or whatever solution is chosen), as attempted in https://github.com/apache/brooklyn-dist/pull/14.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] brooklyn-client pull request: Add -Pno-go-client profile
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the pull request:
https://github.com/apache/brooklyn-client/pull/4#issuecomment-194772372
How about we use the fact that you can deactivate a profile, a la
`mvn groupId:artifactId:goal -P !profile-1,!profile-2`
could we use that to let us
1. Do without the no-op profile
2. Just disable go-client profile when we didn't want it?
We'd need to make the matching change in brooklyn-dist.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---