You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by johnmccabe <gi...@git.apache.org> on 2016/03/22 11:53:18 UTC
[GitHub] brooklyn-client pull request: remove brooklyn-client Godep from re...
GitHub user johnmccabe opened a pull request:
https://github.com/apache/brooklyn-client/pull/11
remove brooklyn-client Godep from repo
@geomacy can you have a look at this, leaving this in results in mvn building from the old Godep. I can build via `mvn`, `godep install`
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/johnmccabe/brooklyn-client fix/godeps
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-client/pull/11.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 #11
----
commit bdb8393472267fb2ae73d044f110ed4da3560068
Author: John McCabe <jo...@johnmccabe.net>
Date: 2016-03-22T10:52:28Z
remove brooklyn-client Godep from repo
- @geomacy can you have a look at this, leaving this in results in mvn building from the old Godep. I can build via `mvn`, `godep install`
----
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199796611
@robertgmoss even for the repo itself? I can see the rationale behind committing dependencies (-ish) but by committing the repo you're either having to update files twice or each update requires 2 commits?
---
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: remove brooklyn-client Godep from re...
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199833278
+1
---
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: remove brooklyn-client Godep from re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-205828973
Thanks @johnmccabe, merging.
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199817027
gonna try reorging it here
---
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: remove brooklyn-client Godep from re...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199781459
:+1: looks good to me, I pulled this and confirm that it builds correctly
---
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: remove brooklyn-client Godep from re...
Posted by neykov <gi...@git.apache.org>.
Github user neykov commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-205780474
Are the changes mentioned by @johnmccabe done ("moving the src up into the br subdirectory should fix the godep behaviour")? Is this PR ready to merge?
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-205783233
@neykov that change wasn't carried out at the time (see [comment](https://github.com/apache/brooklyn-client/pull/11#issuecomment-199831958), the PR as-is allows the client to build as expected pending the 1.6 vendoring support @geomacy plans to add - suggest we merge in its absence.
---
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: remove brooklyn-client Godep from re...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-client/pull/11
---
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: remove brooklyn-client Godep from re...
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199801547
I have a feeling the Godeps.json file has not been created properly - I think it is only supposed to have external dependencies in there...
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199816193
I suspect its due to godeps existing in the br subdirectory rather than in the root of the project
---
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: remove brooklyn-client Godep from re...
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199799131
Ah didn't see that! I thought you were removing everything. Should be fine the way you have it.
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199838264
just had a look at the vendoring in 1.6 and it appears that you're going to hit the same problem with directory structuring
---
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: remove brooklyn-client Godep from re...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199814053
You're quite right @robertgmoss, it shouldn't have references to brooklyn-client itself in it.
---
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: remove brooklyn-client Godep from re...
Posted by geomacy <gi...@git.apache.org>.
Github user geomacy commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199834090
that sounds like a reasonable plan to me. I'd hope to get the 1.6 vendoring support added sooner rather than later, in time for a 0.9.0 release of Brooklyn.
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199831958
moving the src up into the br subdirectory should fix the godep behaviour (moving Godep down also fixes it but you then hit the problem that the repo name doesn't match the br binary).
as @geomacy points out given that there is an intention to move away from godep to the vendoring in 1.6 its probably not worth attempting to make further changes at this point.
this PR in its current form at least ensures the binary is built from the latest code rather than an obsolete copy of the repo, I suggest we commit this and can revisit vendoring as part of the 1.6 move.
---
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: remove brooklyn-client Godep from re...
Posted by robertgmoss <gi...@git.apache.org>.
Github user robertgmoss commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199795833
I believe it is customary in Golang circles to commit the Godep to repos. If the Godep is out of date perhaps it should be updated?
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199814985
currently its the `godep save` thats adding brooklyn-client
---
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: remove brooklyn-client Godep from re...
Posted by johnmccabe <gi...@git.apache.org>.
Github user johnmccabe commented on the pull request:
https://github.com/apache/brooklyn-client/pull/11#issuecomment-199799536
figured :) I was about to bust out my 'I heard you like repos so I put a repo in your repo' meme.
---
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.
---