unity-scope-click

Merge lp://qastaging/~mardy/unity-scope-click/signon-plugin into lp://qastaging/unity-scope-click

  1. signon-plugin
  2. Merge into trunk
Proposed by Alberto Mardegan
Status: Rejected
Rejected by: dobey
Proposed branch: lp://qastaging/~mardy/unity-scope-click/signon-plugin
Merge into: lp://qastaging/unity-scope-click
Diff against target: 52 lines (+11/-4)
2 files modified
debian/control (+1/-1)
libclickscope/click/preview.cpp (+10/-3)
To merge this branch: bzr merge lp://qastaging/~mardy/unity-scope-click/signon-plugin
Reviewer Review Type Date Requested Status
dobey (community) Disapprove
PS Jenkins bot continuous-integration Needs Fixing
Review via email: mp+288758@code.qastaging.launchpad.net

Commit message

Pass the TokenName when authenticating.

Description of the change

Pass the TokenName when authenticating.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:427
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/729/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/133/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/129/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/729/rebuild

review: Needs Fixing (continuous-integration)
lp://qastaging/~mardy/unity-scope-click/signon-plugin updated
428. By Alberto Mardegan

Bump dependency version

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:428
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/730/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/134/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/130/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/730/rebuild

review: Needs Fixing (continuous-integration)
lp://qastaging/~mardy/unity-scope-click/signon-plugin updated
429. By Alberto Mardegan

bump dep

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:429
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/731/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/135/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/131/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/731/rebuild

review: Needs Fixing (continuous-integration)
lp://qastaging/~mardy/unity-scope-click/signon-plugin updated
430. By Alberto Mardegan

From trunk

* Revert the change to sign all requests, as it exposes some
  consistent crashing. (LP: #1563007)
* Must not sign the call to get the existing reviews for a package.
  (LP: #1561997)
[ Antti Kaijanmäki, Rodney Dawes ]
* Fix the signing of store webservice urls (LP: #1483866)
[ Pawel Stolowski ]
* Always push departments (departments should be available also in
  search mode) and allow searching inside departments. (LP: #1541025)
* Add dependency on packagekit so install of clicks will work on PCs.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:430
http://jenkins.qa.ubuntu.com/job/unity-scope-click-ci/734/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-amd64-ci/138/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-scope-click-wily-armhf-ci/134/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity-scope-click-ci/734/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
dobey (dobey) wrote :

Why is this change required?

review: Needs Information
Revision history for this message
Alberto Mardegan (mardy) wrote :

> Why is this change required?

Because the token name is a runtime parameter that depends on the hostname (therefore, we cannot put it in the <template> element of a .service file).
Of course, the root reason is that I made the TokenName a required parameter of the new signon-plugin-ubuntuone authentication plugin; another option is that we make it optional, and make the authentication plugin build the default value itself.

Let me know, if that's how you'd prefer it to be done (I think it's much cleaner like this, but indeed it causes more changes around).

Revision history for this message
dobey (dobey) wrote :

Yes, let's not do this then. It was my understanding from the meeting where we decided to go ahead with implementing the signon plugin, that we would not break existing API, or require changes to the things which use the U1 account, and all the changes would be to ubuntuone-credentials itself.

This seems like unnecessary duplication of code, to me.

review: Disapprove

Unmerged revisions

430. By Alberto Mardegan

From trunk

* Revert the change to sign all requests, as it exposes some
  consistent crashing. (LP: #1563007)
* Must not sign the call to get the existing reviews for a package.
  (LP: #1561997)
[ Antti Kaijanmäki, Rodney Dawes ]
* Fix the signing of store webservice urls (LP: #1483866)
[ Pawel Stolowski ]
* Always push departments (departments should be available also in
  search mode) and allow searching inside departments. (LP: #1541025)
* Add dependency on packagekit so install of clicks will work on PCs.

429. By Alberto Mardegan

bump dep

428. By Alberto Mardegan

Bump dependency version

427. By Alberto Mardegan

Pass token name

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches