search

OpenAgricultureFoundation / openag_brain

ROS package for controlling an OpenAg food computer
GNU General Public License v3.0
221 stars 68 forks source link

openag_brain v1.0.0 release merging the develop branch to the (stable) master. #337

Closed rbaynes closed 7 years ago

rbaynes commented 7 years ago

So changes since June 5! Sorry to ask you to review this @Spaghet 👍

rbaynes commented 7 years ago

Hi David, Please make a PR against the develop branch. Thanks!

On Fri, Aug 25, 2017 at 7:39 PM David notifications@github.com wrote:

@davoclavo commented on this pull request.

In Dockerfile https://github.com/OpenAgInitiative/openag_brain/pull/337#discussion_r135372057 :

@@ -45,15 +45,16 @@ RUN sudo chown -R pi:pi ~/catkin_ws

Install ROS boostrapping tool

RUN sudo apt-get update && sudo apt-get install --no-install-recommends -y -q \ python-pip python-rosdep +

I should probably file a separate issue, but the Dockerfile in develop is currently broken, mainly because it got out of sync with scripts/install_dev. I've got a working version here (summary in the commit message) https://github.com/davoclavo/openag_brain/commit/5f7e97a7d651c388b99227a591d39f64a3d2e1c1 but was trying to see if I could refine it more before submitting a PR, let me know if I should submit one soon before this PR gets merged or if we can push it back for a future release. My changes make the docker build rely on the scripts/install_dev which sets up almost everything; it is not the best way to solve this, but it works in the interim. The reason why I say is not the best solution is that then docker can't leverage caching between commands, so every time a change is made in the install_dev script, the whole step has to be re-built, which takes a long time.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/OpenAgInitiative/openag_brain/pull/337#pullrequestreview-58784539, or mute the thread https://github.com/notifications/unsubscribe-auth/AK6m70GVxxDg-biC_J-HEGswzmr7ubLtks5sb1s5gaJpZM4PC4wI .

--

Thanks, Rob Baynes

rbaynes commented 7 years ago

@Spaghet I need to merge this (develop branch) to master to complete the 1.0.0 release, so I can publish it and GH will do the tagging and we can do the announcement. Could you review? TIA!

sp4ghet commented 7 years ago

Is the docker PR merged?

rbaynes commented 7 years ago

@Spaghet YES, the docker PR has now been tested and is merge to the develop branch. Please review and approve this.