Merge lp:~bkerensa/charms/precise/subway/trunk into lp:charms/subway

Proposed by Benjamin Kerensa on 2012-07-16
Status: Rejected
Rejected by: Charles Butler on 2014-08-29
Proposed branch: lp:~bkerensa/charms/precise/subway/trunk
Merge into: lp:charms/subway
Diff against target: 15 lines (+2/-2)
1 file modified
hooks/install (+2/-2)
To merge this branch: bzr merge lp:~bkerensa/charms/precise/subway/trunk
Reviewer Review Type Date Requested Status
Charles Butler (community) Needs Fixing on 2014-08-29
Mark Mims 2012-07-16 Pending
Review via email: mp+115188@code.launchpad.net

Description of the change

* Replaced PPA with install of nodejs from Ubuntu Archives

To post a comment you must log in.
Kapil Thangavelu (hazmat) wrote :

lgtm

On Mon, Jul 16, 2012 at 1:03 PM, Benjamin Kerensa <email address hidden>wrote:

> Benjamin Kerensa has proposed merging
> lp:~bkerensa/charms/precise/subway/trunk into lp:charms/subway.
>
> Requested reviews:
> Mark Mims (mark-mims)
>
> For more details, see:
>
> https://code.launchpad.net/~bkerensa/charms/precise/subway/trunk/+merge/115188
>
> * Replaced PPA with install of nodejs from Ubuntu Archives
>
> --
>
> https://code.launchpad.net/~bkerensa/charms/precise/subway/trunk/+merge/115188
> Your team charmers is subscribed to branch lp:charms/subway.
>
> === modified file 'hooks/install'
> --- hooks/install 2012-03-23 20:29:39 +0000
> +++ hooks/install 2012-07-16 17:02:19 +0000
> @@ -16,9 +16,9 @@
> juju-log "Installing MongoDB"
> sudo apt-get -y install mongodb
>
> -# Grabbing Nodejs from PPA since Ubuntu Repo is outdated
> +# Installing nodejs
> juju-log "Installing nodejs"
> -sudo add-apt-repository -y ppa:chris-lea/node.js && sudo apt-get update
> -y && sudo apt-get -y install nodejs
> +sudo apt-get -y install nodejs
>
> # Install npm and build-essentials
> juju-log "Installing npm"
>
>
>

Charles Butler (lazypower) wrote :

Benjamin,

Thank you so much for the contribution. I've taken the liberty of reviewing your change, and it appears that the merge has broken functionality. The PPA provided by chris-lea still functions and communicates with NPM. When I merged your changes that remove the PPA, and reverts to the ubuntu archives, communication with the npm distribution server was not functional.

Until such a time arrives that the npm package in teh ubuntu archive is able to communicate with the npmjs server, i cannot approve this merge.

Thanks again for the submission. I'm going to change status of this MP to "needs work" and when you're ready for another review please click the "Request another review" button in the upper right hand corner of the commit message.

If you have any questions/comments/concerns about the review contact us in #juju on irc.freenode.net or email the mailing list <email address hidden>, or ask a question tagged with "juju" on http://askubuntu.com.

review: Needs Fixing

Unmerged revisions

15. By Benjamin Kerensa on 2012-07-16

Replace PPA Nodejs with Archive

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'hooks/install'
2--- hooks/install 2012-03-23 20:29:39 +0000
3+++ hooks/install 2012-07-16 17:02:19 +0000
4@@ -16,9 +16,9 @@
5 juju-log "Installing MongoDB"
6 sudo apt-get -y install mongodb
7
8-# Grabbing Nodejs from PPA since Ubuntu Repo is outdated
9+# Installing nodejs
10 juju-log "Installing nodejs"
11-sudo add-apt-repository -y ppa:chris-lea/node.js && sudo apt-get update -y && sudo apt-get -y install nodejs
12+sudo apt-get -y install nodejs
13
14 # Install npm and build-essentials
15 juju-log "Installing npm"

Subscribers

People subscribed via source and target branches

to all changes: