Date   
Re: Go Lang Mono Repo - clean up and some issue resolution for Tuesday next

Steve Osselton
 

Hi Jim,

Probably not worth the bother of the infrastructure set up for just 0MQ, might have made more sense if
had a lot of binary dependencies, but don't think we do.

Cheers Steve.

On 5 March 2018 at 21:26, <James.White2@...> wrote:

Dell - Internal Use - Confidential

Hi Steve,

We are exploring all sorts of options, but have not considered using a build library.

My first preference is to have something that is at least compiled with the application (Java and Go libs that can be used with their language compilers).  A binary library is an option if we think something like 0MQ is the long term strategy and we really want to maintain our own repo of pre-built binaries.  But we’d still have to create those (for the various platforms) and have the repo hosted somewhere (LF?).  And then we’d have to incorporate that pull of those for developer, docker builds, etc.  Not sure that saves us a ton.   Or am I missing something?

jim

 

From: Steve Osselton [mailto:steve@...]
Sent: Monday, March 05, 2018 8:25 AM
To: White2, James <James_White2@...>
Cc: edgex-golang@lists.edgexfoundry.org
Subject: Re: [Edgex-golang] Go Lang Mono Repo - clean up and some issue resolution for Tuesday next

 

Just a quick thought WRT to 0MQ (and any other library dependency), have we considered using a binary repository for the build library (Conan),

would remove the dependency of having it installed on a build system?

 

Cheers Steve.

 

On 3 March 2018 at 00:11, <James.White2@...> wrote:

All,

Good work this week getting the mono repo and associated builds up and running.  During this week’s Go Project meeting (Tuesday), I’d like to address a few issues I see now that we have a single repo.  Consul being the chief among them

1.       Consul integration:  we need the Go code to operate as closely to the way the Java side works.  It provided for the flexibility of Consul or no-Consul and Docker or no-Docker (and allows for other container strategy later).  Here is what needs to be in the Go micro services:

·         There are two configurations associated to the application (in our core, we have made those external to the artifact in a res folder; in export, the config is in a .go file – I am ok with either approach but I want one for dockerized and one for non-docker environments)

·         Both configurations should also be in config-seed so Consul is prepopulated with the dockerized and non-dockerized config.  I recommend the following profile names:

o   <app-name>;docker;go

o   <app-name>;go

·         A bootstrap profile property specifies which configuration profile to use.  By default containerized properties should be used (profile=docker;go) but it could be set to use non-dockerized (profile=go) [ this is important for things like development where you don’t want to run in a container and some properties have to be set differently]

·         A second bootstrap property should specify whether to even attempt to connect and user Consul or not.  By default, this is set to true (in Java, the property was consul.enabled=true or false).

·         On start, the following algorithm is used…

If (consul.enabled=true)

If (Consul is up and the micro service can get to it)

micro service should register with Consul and get its configuration from Consul the <app-name>; <profile property> configuration profile in Consule

fi

else  # Consul is not up or consul.enabled=false

if (profile=docker;go)

micro service should get its configuration from the res folder or .go file associated to the docker;go configuration profile

else

micro service should get its configuration from the res folder or .go file associated to the ;go configuration profile

fi

fi

2.       In the Java code, the service would report to the log how long it took to start up the service.  Any reason why we can’t do this in Go?  An example is done with Core services?

3.       In the Java code, a heartbeat provided a log entry with a timestamp so that if you are looking at or tailing the logs, you had a sense the service was still alive.  Any reason why we can’t or shouldn’t do this?

4.       The main.go in cmd is consistent for all the Go services.  After that, it differs.  Should we have a consistently named kick-off function/file name for the main.go to call?  This could really help developers follow the line from the cmd/ folder to the micro service folder.

5.       Installing 0MQ is not exactly a straightforward process.  Until we replace it or have an alternate, can we add a script to install 0MQ?  I have referenced one in core data.  We can then reference this from the edgex-go README

6.       Where possible we should make the README’s consistent.  Someone want to take a shot at the README template?  Or pick one from the existing

7.       Should glide.lock be in .gitignore?

Jim White

Distinguished Engineer, IoT Platform Development Team Lead

Dell Technologies | IoT Solutions Division

Office +1 512-723-6139, mobile/text +1 612-916-6693

james_white2@...

 


_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@lists.edgexfoundry.org
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang

 


Go Lang Project Meeting today at 2pm CST

James.White2@...
 

Dell - Internal Use - Confidential

Good day everyone.  A rreminder that the Go Project meets today at 2pm CST. Agenda topics include Mono repo clean up, and messaging. Please join us. You can find the propose agenda and connection information at: https://wiki.edgexfoundry.org/display/FA/Go+Lang+Microservices+Project+Group

 

 

Jim White

Distinguished Engineer, IoT Platform Development Team Lead

Dell Technologies | IoT Solutions Division

Office +1 512-723-6139, mobile/text +1 612-916-6693

james_white2@...

 

Mono repo Makefile changes?

espy
 

Looks like after our meeting last night, a number of outstanding pull requests got merged (including mine; thanks Jim!).

That said, my snap build breaks in a different manner this morning due to a Makefile change.

I was building all of the services in the mono repo using the following commands:

make prepare
make build

This no longer works. I traced the change to this PR:

https://github.com/edgexfoundry/edgex-go/pull/28

My snap is building now, as I changed my build to use:

make prepare
make build_microservices

The PR explains why the change was made, but doesn't mention that 'make build' no longer builds the microservices.

In the future, if we change Makefile semantics, an email to the list would be helpful, and I would also ask that the associated commit message offer a bit more context. The associated commit with this change was simply:

"Add new target to compile all go code"

Also I would've thought we'd want to keep "build" as a target for building all of the code in the repo? It looks like none of our make targets are using any kind of prequisites, which could've been used to ensure proper ordering of the build. It appears we're relying on both go and make for our builds, and thus the semantics of how the build actually works are a bit blurry.

Finally, the current clean target only appears to clean $(MICROSERVICES), so this will need to be fixed.

Thoughts?

Regards,
/tony

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

On Wed, Mar 7, 2018 at 3:39 PM, espy <espy@...> wrote:

In the future, if we change Makefile semantics, an email to the list would
be helpful
I think that the best thing is to subscribe yourself to edgex-go repo
("watch" feature) and go through PRs, comment and review them as they
are published. There is not so many of them (unfortunately), 2-3 per
day max.

Also, always when you encounter the issue - please open one with the
description on GitHub, so we can comment on it.

BR,
Drasko

Re: Mono repo Makefile changes?

espy
 

On 03/07/2018 09:48 AM, Drasko DRASKOVIC wrote:
On Wed, Mar 7, 2018 at 3:39 PM, espy <espy@...> wrote:

In the future, if we change Makefile semantics, an email to the list would
be helpful
I think that the best thing is to subscribe yourself to edgex-go repo
("watch" feature) and go through PRs, comment and review them as they
are published. There is not so many of them (unfortunately), 2-3 per
day max.
Sure, I can do that, however doing so doesn't address any of the other issues I raised in my email. There's a bit more context in the PR description, but still no mention that the default build target changed.

Also, always when you encounter the issue - please open one with the
description on GitHub, so we can comment on it.
Sure, or I'll submit a patch. ;)-

Regards,
/tony

Re: Mono repo Makefile changes?

Jeremy Phelps
 

Another thing to keep in mind here is that the verify jobs in Jenkins make "make calls".  Updating the Makefile is good but please PR the ci-managment repo with an update if the make invocation changes too.
Jeremy

On Wed, Mar 7, 2018 at 8:51 AM, espy <espy@...> wrote:
On 03/07/2018 09:48 AM, Drasko DRASKOVIC wrote:
On Wed, Mar 7, 2018 at 3:39 PM, espy <espy@...> wrote:

In the future, if we change Makefile semantics, an email to the list would
be helpful

I think that the best thing is to subscribe yourself to edgex-go repo
("watch" feature) and go through PRs, comment and review them as they
are published. There is not so many of them (unfortunately), 2-3 per
day max.

Sure, I can do that, however doing so doesn't address any of the other issues I raised in my email.  There's a bit more context in the PR description, but still no mention that the default build target changed.

Also, always when you encounter the issue - please open one with the
description on GitHub, so we can comment on it.

Sure, or I'll submit a patch.  ;)-

Regards,
/tony


_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@...y.org
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

On Wed, Mar 7, 2018 at 3:51 PM, espy <espy@...> wrote:
On 03/07/2018 09:48 AM, Drasko DRASKOVIC wrote:

On Wed, Mar 7, 2018 at 3:39 PM, espy <espy@...> wrote:

In the future, if we change Makefile semantics, an email to the list
would
be helpful

I think that the best thing is to subscribe yourself to edgex-go repo
("watch" feature) and go through PRs, comment and review them as they
are published. There is not so many of them (unfortunately), 2-3 per
day max.

Sure, I can do that, however doing so doesn't address any of the other
issues I raised in my email. There's a bit more context in the PR
description, but still no mention that the default build target changed.
No need for these trivial changes IMHO - context can be easily seen
from the code: https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24


Also, always when you encounter the issue - please open one with the
description on GitHub, so we can comment on it.

Sure, or I'll submit a patch. ;)-
Even better ;).

BR,
Drasko

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

On Wed, Mar 7, 2018 at 3:55 PM, Jeremy Phelps
<jphelps@...> wrote:
Another thing to keep in mind here is that the verify jobs in Jenkins make
"make calls". Updating the Makefile is good but please PR the ci-managment
repo with an update if the make invocation changes too.
Isn't CI run upon new (patched) Makefile? If it passes it should be OK, right?

BR,
Drasko

Re: Mono repo Makefile changes?

Jeremy Phelps
 

It depends on how the make targets are changed.  It may require updates the the edgex-go* scripts here https://github.com/edgexfoundry/ci-management/tree/master/shell.  For example check out how they call make targets currently.
Jeremy

On Wed, Mar 7, 2018 at 8:59 AM, Drasko DRASKOVIC <drasko.draskovic@...> wrote:
On Wed, Mar 7, 2018 at 3:55 PM, Jeremy Phelps
<jphelps@...> wrote:
> Another thing to keep in mind here is that the verify jobs in Jenkins make
> "make calls".  Updating the Makefile is good but please PR the ci-managment
> repo with an update if the make invocation changes too.

Isn't CI run upon new (patched) Makefile? If it passes it should be OK, right?

BR,
Drasko

Re: Mono repo Makefile changes?

Steve Osselton
 

Broke my arm 32 bit build as well. e-mail head's up would have been nice ...

On 7 March 2018 at 14:39, espy <espy@...> wrote:
Looks like after our meeting last night, a number of outstanding pull requests got merged (including mine; thanks Jim!).

That said, my snap build breaks in a different manner this morning due to a Makefile change.

I was building all of the services in the mono repo using the following commands:

make prepare
make build

This no longer works.  I traced the change to this PR:

https://github.com/edgexfoundry/edgex-go/pull/28

My snap is building now, as I changed my build to use:

make prepare
make build_microservices

The PR explains why the change was made, but doesn't mention that 'make build' no longer builds the microservices.

In the future, if we change Makefile semantics, an email to the list would be helpful, and I would also ask that the associated commit message offer a bit more context.  The associated commit with this change was simply:

"Add new target to compile all go code"

Also I would've thought we'd want to keep "build" as a target for building all of the code in the repo?  It looks like none of our make targets are using any kind of prequisites, which could've been used to ensure proper ordering of the build.  It appears we're relying on both go and make for our builds, and thus the semantics of how the build actually works are a bit blurry.

Finally, the current clean target only appears to clean $(MICROSERVICES), so this will need to be fixed.

Thoughts?

Regards,
/tony

_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@...y.org
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang



--
Technical Director
IOTech Systems Ltd.

Re: Mono repo Makefile changes?

James.White2@...
 

Dell - Internal Use - Confidential

All

We cannot – we will not!!! – expect developers to read through all the PRs to be able to pull down and work with EdgeX and have a proper build.  This is not a way to run a project.

 

If the build breaks and it is a result of something you added – the responsibility is on the developer that caused the break to put a new PR in place to fix it or reverse it as quickly as possible.

If you have something you think needs review/comments then I think we need to use branches and seek comments via this email list, Rocket Chat, etc.

 

IF there are extenuating circumstances for a change to something like a Makefile or any other elements that we know will cause a break – we absolutely need a heads up BEFORE it breaks.  In fact, do so and get the ok from the community before so that you do not interrupt the course of business.

 

Drasko – what does it take to get a fix in place ASAP??

 

Jim

 

From: edgex-golang-bounces@... [mailto:edgex-golang-bounces@...] On Behalf Of Steve Osselton
Sent: Wednesday, March 07, 2018 9:08 AM
To: espy <espy@...>
Cc: edgex-golang@...
Subject: Re: [Edgex-golang] Mono repo Makefile changes?

 

Broke my arm 32 bit build as well. e-mail head's up would have been nice ...

 

On 7 March 2018 at 14:39, espy <espy@...> wrote:

Looks like after our meeting last night, a number of outstanding pull requests got merged (including mine; thanks Jim!).

That said, my snap build breaks in a different manner this morning due to a Makefile change.

I was building all of the services in the mono repo using the following commands:

make prepare
make build

This no longer works.  I traced the change to this PR:

https://github.com/edgexfoundry/edgex-go/pull/28

My snap is building now, as I changed my build to use:

make prepare
make build_microservices

The PR explains why the change was made, but doesn't mention that 'make build' no longer builds the microservices.

In the future, if we change Makefile semantics, an email to the list would be helpful, and I would also ask that the associated commit message offer a bit more context.  The associated commit with this change was simply:

"Add new target to compile all go code"

Also I would've thought we'd want to keep "build" as a target for building all of the code in the repo?  It looks like none of our make targets are using any kind of prequisites, which could've been used to ensure proper ordering of the build.  It appears we're relying on both go and make for our builds, and thus the semantics of how the build actually works are a bit blurry.

Finally, the current clean target only appears to clean $(MICROSERVICES), so this will need to be fixed.

Thoughts?

Regards,
/tony

_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@...
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang



 

--

Technical Director

IOTech Systems Ltd.

Re: Mono repo Makefile changes?

espy
 

On 03/07/2018 11:19 AM, James.White2@... wrote:
*Dell - Internal Use - Confidential
*
All
We cannot – we will not!!! – expect developers to read through all the PRs to be able to pull down and work with EdgeX and have a proper build.  This is not a way to run a project.
If the build breaks and it is a result of something you added – the responsibility is on the developer that caused the break to put a new PR in place to fix it or reverse it as quickly as possible.
If you have something you think needs review/comments then I think we need to use branches and seek comments via this email list, Rocket Chat, etc.
IF there are extenuating circumstances for a change to something like a Makefile or any other elements that we know will cause a break – we absolutely need a heads up *_BEFORE_* it breaks.  In fact, do so and get the ok from the community before so that you do not interrupt the course of business.
*Drasko*– what does it take to get a fix in place ASAP??
Just to be clear, I'm unblocked at the moment, however I do think it would make sense for us to take a good look at the current Makefile approach and re-factor if/where appropriate.

Regards,
/tony


Jim
*From:*edgex-golang-bounces@... [mailto:edgex-golang-bounces@...] *On Behalf Of *Steve Osselton
*Sent:* Wednesday, March 07, 2018 9:08 AM
*To:* espy <espy@...>
*Cc:* edgex-golang@...
*Subject:* Re: [Edgex-golang] Mono repo Makefile changes?
Broke my arm 32 bit build as well. e-mail head's up would have been nice ...
On 7 March 2018 at 14:39, espy <espy@... <mailto:espy@...>> wrote:
Looks like after our meeting last night, a number of outstanding
pull requests got merged (including mine; thanks Jim!).
That said, my snap build breaks in a different manner this morning
due to a Makefile change.
I was building all of the services in the mono repo using the
following commands:
make prepare
make build
This no longer works.  I traced the change to this PR:
https://github.com/edgexfoundry/edgex-go/pull/28
My snap is building now, as I changed my build to use:
make prepare
make build_microservices
The PR explains why the change was made, but doesn't mention that
'make build' no longer builds the microservices.
In the future, if we change Makefile semantics, an email to the list
would be helpful, and I would also ask that the associated commit
message offer a bit more context.  The associated commit with this
change was simply:
"Add new target to compile all go code"
Also I would've thought we'd want to keep "build" as a target for
building all of the code in the repo?  It looks like none of our
make targets are using any kind of prequisites, which could've been
used to ensure proper ordering of the build.  It appears we're
relying on both go and make for our builds, and thus the semantics
of how the build actually works are a bit blurry.
Finally, the current clean target only appears to clean
$(MICROSERVICES), so this will need to be fixed.
Thoughts?
Regards,
/tony
_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@...
<mailto:EdgeX-GoLang@...>
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang
--
Technical Director
IOTech Systems Ltd.

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

Hi Jim,

On Wed, Mar 7, 2018 at 5:19 PM, <James.White2@...> wrote:
Dell - Internal Use - Confidential

All

We cannot – we will not!!! – expect developers to read through all the PRs
to be able to pull down and work with EdgeX and have a proper build. This
is not a way to run a project.
I do not expect all developers to go through PRs, but I expect
reviewers and maintainers to do it. I am not a maintainer (but never
the less I do my best to do code reviews). You have approved this PR
even before I did a review
(https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you
also reviewed it.



If the build breaks and it is a result of something you added – the
responsibility is on the developer that caused the break to put a new PR in
place to fix it or reverse it as quickly as possible.
This can be done by the maintainer as well, or reviewer - who equally
share the responsibility that this code was merged to mainstream.
Developer usually comes with best intent, locally tested code and
relays on reviewers and maintainers, and CI (which in this case
failed) to prohibit breaking errors.


If you have something you think needs review/comments then I think we need
to use branches and seek comments via this email list, Rocket Chat, etc.
GitHub notifications are not enough?


IF there are extenuating circumstances for a change to something like a
Makefile or any other elements that we know will cause a break – we
absolutely need a heads up BEFORE it breaks. In fact, do so and get the ok
from the community before so that you do not interrupt the course of
business.
Everybody get notifications on every PR submitted. Then everybody gets
additional notifications upon every review posted. Then everybody gets
notifications upon maintainer approval. And finally everybody gets
notifications when code is merged. And all this is not enough and you
want me to send the mail to the list?

Drasko – what does it take to get a fix in place ASAP??
Why is my name in red here? I did not post this PR, I did not merge it
(I am not a maintainer), I did not set CI that can't detect the
compilation errors.

Problem is coming probably from this lilne:
https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
I am not sure why ARM build did not like `novendor`. But without error
logs and issue opened it will not be easy (our SW is not
cross-compiled on the host, as I was proposing before, but naively
built on ARM machines, and now I have to find ARM machine to test).

Best regards,
Drasko

Re: Mono repo Makefile changes?

Jeremy Phelps
 

In light of all this, I think that it would be better for me to make the CI not dependent on the Makefile structure.  This will let devs and committers manage it as they see fit and for easy developer upstart without having to worry about breaking CI (there are a couple cases in this scenario where CI would give the go-ahead when all is not good).  I will refactor, but in the meantime the verify jobs absolutely depend on there being the following calls:

make prepare
make test
make build
make docker

On Wed, Mar 7, 2018 at 4:05 PM, Drasko DRASKOVIC <drasko.draskovic@...> wrote:
Hi Jim,

On Wed, Mar 7, 2018 at 5:19 PM,  <James.White2@...> wrote:
> Dell - Internal Use - Confidential
>
> All
>
> We cannot – we will not!!! – expect developers to read through all the PRs
> to be able to pull down and work with EdgeX and have a proper build.  This
> is not a way to run a project.

I do not expect all developers to go through PRs, but I expect
reviewers and maintainers to do it. I am not a maintainer (but never
the less I do my best to do code reviews). You have approved this PR
even before I did a review
(https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you
also reviewed it.


>
> If the build breaks and it is a result of something you added – the
> responsibility is on the developer that caused the break to put a new PR in
> place to fix it or reverse it as quickly as possible.

This can be done by the maintainer as well, or reviewer - who equally
share the responsibility that this code was merged to mainstream.
Developer usually comes with best intent, locally tested code and
relays on reviewers and maintainers, and CI (which in this case
failed) to prohibit breaking errors.

>
> If you have something you think needs review/comments then I think we need
> to use branches and seek comments via this email list, Rocket Chat, etc.

GitHub notifications are not enough?

>
> IF there are extenuating circumstances for a change to something like a
> Makefile or any other elements that we know will cause a break – we
> absolutely need a heads up BEFORE it breaks.  In fact, do so and get the ok
> from the community before so that you do not interrupt the course of
> business.

Everybody get notifications on every PR submitted. Then everybody gets
additional notifications upon every review posted. Then everybody gets
notifications upon maintainer approval. And finally everybody gets
notifications when code is merged. And all this is not enough and you
want me to send the mail to the list?

>  Drasko – what does it take to get a fix in place ASAP??

Why is my name in red here? I did not post this PR, I did not merge it
(I am not a maintainer), I did not set CI that can't detect the
compilation errors.

Problem is coming probably from this lilne:
https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
I am not sure why ARM build did not like `novendor`. But without error
logs and issue opened it will not be easy (our SW is not
cross-compiled on the host, as I was proposing before, but naively
built on ARM machines, and now I have to find ARM machine to test).

Best regards,
Drasko
_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@lists.edgexfoundry.org
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

On Wed, Mar 7, 2018 at 5:37 PM, espy <espy@...> wrote:
On 03/07/2018 11:19 AM, James.White2@... wrote:

*Dell - Internal Use - Confidential

*

All

We cannot – we will not!!! – expect developers to read through all the PRs
to be able to pull down and work with EdgeX and have a proper build. This
is not a way to run a project.

If the build breaks and it is a result of something you added – the
responsibility is on the developer that caused the break to put a new PR in
place to fix it or reverse it as quickly as possible.

If you have something you think needs review/comments then I think we need
to use branches and seek comments via this email list, Rocket Chat, etc.

IF there are extenuating circumstances for a change to something like a
Makefile or any other elements that we know will cause a break – we
absolutely need a heads up *_BEFORE_* it breaks. In fact, do so and get the
ok from the community before so that you do not interrupt the course of
business.

*Drasko*– what does it take to get a fix in place ASAP??

Just to be clear, I'm unblocked at the moment, however I do think it would
make sense for us to take a good look at the current Makefile approach and
re-factor if/where appropriate.
I would also like to add that I would expect that every developer is
capable of doing `git chechout <git_hash_that_works>` and build the
SW. If you can not do this, you are probably not a developer and you
should not be building the SW in the first place, but downloading and
using pre-compiled binaries or whatever.

There is not even tagged MVP on this repo, not to mention an official
release. With all the effort that developers, CI, reviewers and
maintainers do, there will be some very rear occasions where master
might be broken. Checking out previous version and rewinding Git HEAD
to a previous commit (working one) is trivial, and should not block a
developer.

As soon as code stabilizes we can tag a release candidate v1.0.0-rc1.0
and release the binaries.

BR,
Drasko

Re: Mono repo Makefile changes?

James.White2@...
 

Dell - Internal Use - Confidential

Drasko,
Deepest apologies as it was my understanding (wrongly) that you had made the submission that caused the break. As you correctly point out, that was not correct. That was inappropriate of me for not looking deeper into the information.

Yes, I agree that developers come with the best intent and occasionally they will create a bug. And so do maintainers/reviewers come with best intent - it is impossible to scrutinize every line of code for all possible issues. So breaks will happen. We want to minimize those, obviously, but I don't expect the system to be issue free. When that happens, I do think we need to go back to the developer and ask that they fix the problem caused as soon as possible - or at least restore to the original until it can be fixed. Given the mono repo, PRs that stand out too long, breaks in the build, etc. all have a bigger impact and can delay others getting their work done. So we have to be sensitive to that. In the discussion chain about this, there was information implying developers need to go back and look at the PRs to understand the issues and state of code. No matter the number of PRs, this is not going to work for the normal developer and is intrusive to their work stream. Mistakes will happen, but let's not punish the person who hits the mistake to be the one that has to dig out all the information on the PRs.

However, I inappropriately applied your comments with regard to PR search to that of the responsible party for the original bug that broke the build. ON that, I apologize and should not have drawn attention to you as the bug creator to fix it. My rash reaction that I shall try not to repeat.

-----Original Message-----
From: Drasko DRASKOVIC [mailto:drasko.draskovic@...]
Sent: Wednesday, March 07, 2018 4:06 PM
To: White2, James <James_White2@...>
Cc: steve@...; espy <espy@...>; Keith Steele <keith@...>; edgex-golang@...
Subject: Re: [Edgex-golang] Mono repo Makefile changes?

Hi Jim,

On Wed, Mar 7, 2018 at 5:19 PM, <James.White2@...> wrote:
Dell - Internal Use - Confidential

All

We cannot – we will not!!! – expect developers to read through all the
PRs to be able to pull down and work with EdgeX and have a proper
build. This is not a way to run a project.
I do not expect all developers to go through PRs, but I expect reviewers and maintainers to do it. I am not a maintainer (but never the less I do my best to do code reviews). You have approved this PR even before I did a review
(https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you also reviewed it.



If the build breaks and it is a result of something you added – the
responsibility is on the developer that caused the break to put a new
PR in place to fix it or reverse it as quickly as possible.
This can be done by the maintainer as well, or reviewer - who equally share the responsibility that this code was merged to mainstream.
Developer usually comes with best intent, locally tested code and relays on reviewers and maintainers, and CI (which in this case
failed) to prohibit breaking errors.


If you have something you think needs review/comments then I think we
need to use branches and seek comments via this email list, Rocket Chat, etc.
GitHub notifications are not enough?


IF there are extenuating circumstances for a change to something like
a Makefile or any other elements that we know will cause a break – we
absolutely need a heads up BEFORE it breaks. In fact, do so and get
the ok from the community before so that you do not interrupt the
course of business.
Everybody get notifications on every PR submitted. Then everybody gets additional notifications upon every review posted. Then everybody gets notifications upon maintainer approval. And finally everybody gets notifications when code is merged. And all this is not enough and you want me to send the mail to the list?

Drasko – what does it take to get a fix in place ASAP??
Why is my name in red here? I did not post this PR, I did not merge it (I am not a maintainer), I did not set CI that can't detect the compilation errors.

Problem is coming probably from this lilne:
https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
I am not sure why ARM build did not like `novendor`. But without error logs and issue opened it will not be easy (our SW is not cross-compiled on the host, as I was proposing before, but naively built on ARM machines, and now I have to find ARM machine to test).

Best regards,
Drasko

Re: Mono repo Makefile changes?

James.White2@...
 

Dell - Internal Use - Confidential

Thanks for the suggestion Jeremy.  Allow me to put this on the agenda for tomorrow’s working group meeting Jeremy.  I am not opposed to this idea, but I can also see that our Makefile is (for better or worse) a key component of our Go development and it needs to work as intended (at least for the basic install & build type work streams).  So I could be persuaded that a properly working Makefile should be part of the CI.  Let’s get community weigh-in and then all be aware of the decision and repercussions.  

jim

 

From: Jeremy Phelps [mailto:jphelps@...]
Sent: Wednesday, March 07, 2018 4:16 PM
To: Drasko DRASKOVIC <drasko.draskovic@...>
Cc: White2, James <James_White2@...>; Keith Steele <keith@...>; edgex-golang@...
Subject: Re: [Edgex-golang] Mono repo Makefile changes?

 

In light of all this, I think that it would be better for me to make the CI not dependent on the Makefile structure.  This will let devs and committers manage it as they see fit and for easy developer upstart without having to worry about breaking CI (there are a couple cases in this scenario where CI would give the go-ahead when all is not good).  I will refactor, but in the meantime the verify jobs absolutely depend on there being the following calls:

make prepare

make test

make build

make docker

 

On Wed, Mar 7, 2018 at 4:05 PM, Drasko DRASKOVIC <drasko.draskovic@...> wrote:

Hi Jim,

On Wed, Mar 7, 2018 at 5:19 PM,  <James.White2@...> wrote:
> Dell - Internal Use - Confidential
>
> All
>
> We cannot – we will not!!! – expect developers to read through all the PRs
> to be able to pull down and work with EdgeX and have a proper build.  This
> is not a way to run a project.

I do not expect all developers to go through PRs, but I expect
reviewers and maintainers to do it. I am not a maintainer (but never
the less I do my best to do code reviews). You have approved this PR
even before I did a review
(https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you
also reviewed it.


>
> If the build breaks and it is a result of something you added – the
> responsibility is on the developer that caused the break to put a new PR in
> place to fix it or reverse it as quickly as possible.

This can be done by the maintainer as well, or reviewer - who equally
share the responsibility that this code was merged to mainstream.
Developer usually comes with best intent, locally tested code and
relays on reviewers and maintainers, and CI (which in this case
failed) to prohibit breaking errors.

>
> If you have something you think needs review/comments then I think we need
> to use branches and seek comments via this email list, Rocket Chat, etc.

GitHub notifications are not enough?

>
> IF there are extenuating circumstances for a change to something like a
> Makefile or any other elements that we know will cause a break – we
> absolutely need a heads up BEFORE it breaks.  In fact, do so and get the ok
> from the community before so that you do not interrupt the course of
> business.

Everybody get notifications on every PR submitted. Then everybody gets
additional notifications upon every review posted. Then everybody gets
notifications upon maintainer approval. And finally everybody gets
notifications when code is merged. And all this is not enough and you
want me to send the mail to the list?

>  Drasko – what does it take to get a fix in place ASAP??

Why is my name in red here? I did not post this PR, I did not merge it
(I am not a maintainer), I did not set CI that can't detect the
compilation errors.

Problem is coming probably from this lilne:
https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
I am not sure why ARM build did not like `novendor`. But without error
logs and issue opened it will not be easy (our SW is not
cross-compiled on the host, as I was proposing before, but naively
built on ARM machines, and now I have to find ARM machine to test).

Best regards,
Drasko

_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@...
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang

 

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

No problem Jim,
really no need to apologize. I do take my part of responsibility for
reviewing positively this commit - but it looks good indeed. I still
do not understand why CI did not break on existing target `build` that
was slightly changed.

Can somebody with ARM machine on his disposal please open an issue and
paste the error logs?

BR,
Drasko

On Wed, Mar 7, 2018 at 11:46 PM, <James.White2@...> wrote:
Dell - Internal Use - Confidential

Drasko,
Deepest apologies as it was my understanding (wrongly) that you had made the submission that caused the break. As you correctly point out, that was not correct. That was inappropriate of me for not looking deeper into the information.

Yes, I agree that developers come with the best intent and occasionally they will create a bug. And so do maintainers/reviewers come with best intent - it is impossible to scrutinize every line of code for all possible issues. So breaks will happen. We want to minimize those, obviously, but I don't expect the system to be issue free. When that happens, I do think we need to go back to the developer and ask that they fix the problem caused as soon as possible - or at least restore to the original until it can be fixed. Given the mono repo, PRs that stand out too long, breaks in the build, etc. all have a bigger impact and can delay others getting their work done. So we have to be sensitive to that. In the discussion chain about this, there was information implying developers need to go back and look at the PRs to understand the issues and state of code. No matter the number of PRs, this is not going to work for the normal developer and is intrusive to their work stream. Mistakes will happen, but let's not punish the person who hits the mistake to be the one that has to dig out all the information on the PRs.

However, I inappropriately applied your comments with regard to PR search to that of the responsible party for the original bug that broke the build. ON that, I apologize and should not have drawn attention to you as the bug creator to fix it. My rash reaction that I shall try not to repeat.

-----Original Message-----
From: Drasko DRASKOVIC [mailto:drasko.draskovic@...]
Sent: Wednesday, March 07, 2018 4:06 PM
To: White2, James <James_White2@...>
Cc: steve@...; espy <espy@...>; Keith Steele <keith@...>; edgex-golang@...
Subject: Re: [Edgex-golang] Mono repo Makefile changes?

Hi Jim,

On Wed, Mar 7, 2018 at 5:19 PM, <James.White2@...> wrote:
Dell - Internal Use - Confidential

All

We cannot – we will not!!! – expect developers to read through all the
PRs to be able to pull down and work with EdgeX and have a proper
build. This is not a way to run a project.
I do not expect all developers to go through PRs, but I expect reviewers and maintainers to do it. I am not a maintainer (but never the less I do my best to do code reviews). You have approved this PR even before I did a review
(https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you also reviewed it.



If the build breaks and it is a result of something you added – the
responsibility is on the developer that caused the break to put a new
PR in place to fix it or reverse it as quickly as possible.
This can be done by the maintainer as well, or reviewer - who equally share the responsibility that this code was merged to mainstream.
Developer usually comes with best intent, locally tested code and relays on reviewers and maintainers, and CI (which in this case
failed) to prohibit breaking errors.


If you have something you think needs review/comments then I think we
need to use branches and seek comments via this email list, Rocket Chat, etc.
GitHub notifications are not enough?


IF there are extenuating circumstances for a change to something like
a Makefile or any other elements that we know will cause a break – we
absolutely need a heads up BEFORE it breaks. In fact, do so and get
the ok from the community before so that you do not interrupt the
course of business.
Everybody get notifications on every PR submitted. Then everybody gets additional notifications upon every review posted. Then everybody gets notifications upon maintainer approval. And finally everybody gets notifications when code is merged. And all this is not enough and you want me to send the mail to the list?

Drasko – what does it take to get a fix in place ASAP??
Why is my name in red here? I did not post this PR, I did not merge it (I am not a maintainer), I did not set CI that can't detect the compilation errors.

Problem is coming probably from this lilne:
https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
I am not sure why ARM build did not like `novendor`. But without error logs and issue opened it will not be easy (our SW is not cross-compiled on the host, as I was proposing before, but naively built on ARM machines, and now I have to find ARM machine to test).

Best regards,
Drasko

Re: Mono repo Makefile changes?

Jeremy Phelps
 

On Wed, Mar 7, 2018 at 5:00 PM, Drasko DRASKOVIC <drasko.draskovic@...> wrote:
No problem Jim,
really no need to apologize. I do take my part of responsibility for
reviewing positively this commit - but it looks good indeed. I still
do not understand why CI did not break on existing target `build` that
was slightly changed.

Can somebody with ARM machine on his disposal please open an issue and
paste the error logs?

BR,
Drasko

On Wed, Mar 7, 2018 at 11:46 PM,  <James.White2@...> wrote:
> Dell - Internal Use - Confidential
>
> Drasko,
> Deepest apologies as it was my understanding (wrongly) that you had made the submission that caused the break.  As you correctly point out, that was not correct.  That was inappropriate of me for not looking deeper into the information.
>
> Yes, I agree that developers come with the best intent and occasionally they will create a bug.  And so do maintainers/reviewers come with best intent - it is impossible to scrutinize every line of code for all possible issues.  So breaks will happen.  We want to minimize those, obviously, but I don't expect the system to be issue free.  When that happens, I do think we need to go back to the developer and ask that they fix the problem caused as soon as possible - or at least restore to the original until it can be fixed.  Given the mono repo, PRs that stand out too long, breaks in the build, etc. all have a bigger impact and can delay others getting their work done.  So we have to be sensitive to that.  In the discussion chain about this, there was information implying developers need to go back and look at the PRs to understand the issues and state of code.  No matter the number of PRs, this is not going to work for the normal developer and is intrusive to their work stream.   Mistakes will happen, but let's not punish the person who hits the mistake to be the one that has to dig out all the information on the PRs.
>
> However, I inappropriately applied your comments with regard to PR search to that of the responsible party for the original bug that broke the build.  ON that, I apologize and should not have drawn attention to you as the bug creator to fix it.  My rash reaction that I shall try not to repeat.
>
> -----Original Message-----
> From: Drasko DRASKOVIC [mailto:drasko.draskovic@gmail.com]
> Sent: Wednesday, March 07, 2018 4:06 PM
> To: White2, James <James_White2@...>
> Cc: steve@...; espy <espy@...>; Keith Steele <keith@...>; edgex-golang@lists.edgexfoundry.org
> Subject: Re: [Edgex-golang] Mono repo Makefile changes?
>
> Hi Jim,
>
> On Wed, Mar 7, 2018 at 5:19 PM,  <James.White2@...> wrote:
>> Dell - Internal Use - Confidential
>>
>> All
>>
>> We cannot – we will not!!! – expect developers to read through all the
>> PRs to be able to pull down and work with EdgeX and have a proper
>> build.  This is not a way to run a project.
>
> I do not expect all developers to go through PRs, but I expect reviewers and maintainers to do it. I am not a maintainer (but never the less I do my best to do code reviews). You have approved this PR even before I did a review
> (https://github.com/edgexfoundry/edgex-go/pull/28) - so I guess you also reviewed it.
>
>
>>
>> If the build breaks and it is a result of something you added – the
>> responsibility is on the developer that caused the break to put a new
>> PR in place to fix it or reverse it as quickly as possible.
>
> This can be done by the maintainer as well, or reviewer - who equally share the responsibility that this code was merged to mainstream.
> Developer usually comes with best intent, locally tested code and relays on reviewers and maintainers, and CI (which in this case
> failed) to prohibit breaking errors.
>
>>
>> If you have something you think needs review/comments then I think we
>> need to use branches and seek comments via this email list, Rocket Chat, etc.
>
> GitHub notifications are not enough?
>
>>
>> IF there are extenuating circumstances for a change to something like
>> a Makefile or any other elements that we know will cause a break – we
>> absolutely need a heads up BEFORE it breaks.  In fact, do so and get
>> the ok from the community before so that you do not interrupt the
>> course of business.
>
> Everybody get notifications on every PR submitted. Then everybody gets additional notifications upon every review posted. Then everybody gets notifications upon maintainer approval. And finally everybody gets notifications when code is merged. And all this is not enough and you want me to send the mail to the list?
>
>>  Drasko – what does it take to get a fix in place ASAP??
>
> Why is my name in red here? I did not post this PR, I did not merge it (I am not a maintainer), I did not set CI that can't detect the compilation errors.
>
> Problem is coming probably from this lilne:
> https://github.com/edgexfoundry/edgex-go/pull/28/files#diff-b67911656ef5d18c4ae36cb6741b7965R24,
> I am not sure why ARM build did not like `novendor`. But without error logs and issue opened it will not be easy (our SW is not cross-compiled on the host, as I was proposing before, but naively built on ARM machines, and now I have to find ARM machine to test).
>
> Best regards,
> Drasko
_______________________________________________
EdgeX-GoLang mailing list
EdgeX-GoLang@lists.edgexfoundry.org
https://lists.edgexfoundry.org/mailman/listinfo/edgex-golang

Re: Mono repo Makefile changes?

Drasko DRASKOVIC <drasko.draskovic@...>
 

On Thu, Mar 8, 2018 at 12:00 AM, Drasko DRASKOVIC
<drasko.draskovic@...> wrote:
No problem Jim,
really no need to apologize. I do take my part of responsibility for
reviewing positively this commit - but it looks good indeed. I still
do not understand why CI did not break on existing target `build` that
was slightly changed.

Can somebody with ARM machine on his disposal please open an issue and
paste the error logs?
Argh... I just saw that we merged a PR even though CI was clearly
stating that ARM build is failing:
https://github.com/edgexfoundry/edgex-go/pull/28

you can see this by clicking on "View Details", bottom right next to
merge message.

Here is the error log:
https://jenkins.edgexfoundry.org/job/edgex-go-master-verify-go-arm/69/console

BR,
Drasko