Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stagingapi: update_status_comments() include link to dashboard. #683

Merged

Conversation

@jberry-suse
Copy link
Contributor

jberry-suse commented Feb 10, 2017

Requested in #573.

Fully tested by running:

api.update_status_comments('openSUSE:Leap:42.3:Staging:adi:9', 'select')

Which resulted in the correct link in comment.

I debated if the link should be placed around a new piece of text like (dashboard) or somesuch, but given the comment is on the project page the only other relevant link related to just the project seems like dashboard. Either way easy to change since url is correct.

The new extract_staging_short() from request_splitter did the trick and works for both letter and adi. :)

@jberry-suse jberry-suse requested a review from lnussel Feb 10, 2017
@coveralls

This comment has been minimized.

Copy link

coveralls commented Feb 10, 2017

Coverage Status

Coverage increased (+0.1%) to 45.489% when pulling 55c4382 on jberry-suse:update_status_comments-dashboard into 6eefb42 on openSUSE:master.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 10, 2017

Could potentially drop the apiurl and use a path relative to root (this assumes the server referenced by apiurl is not hosted in subdir (which I am not sure if OBS even supports)). The url would just be /project/staging_projects/... instead of http://..../project/staging_projects/....

@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Feb 14, 2017

the adi seems gone again. apiurl would be api.o.o, right? We need build.o.o

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 14, 2017

I remember reading the code in obs_factory, and in practice it seems to work since the plugin redirect internally for paths that are part of the application and not API. I can change it, but then I imagine it either needs to be hardcoded or derived from the apiurl which is potentially b.o.c specific. Long-story short api.o.c works.

Alternatively the relative path describe in my previous comment.

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 14, 2017

@jberry-suse

This comment has been minimized.

Copy link
Contributor Author

jberry-suse commented Feb 14, 2017

I just confirmed the relative approach works as well so perhaps just use that?

[text](/project/staging_projects/openSUSE:Leap:42.3/A)
@lnussel

This comment has been minimized.

Copy link
Member

lnussel commented Feb 15, 2017

ah, the api link works if logged in. I don't mind either way.

@lnussel lnussel merged commit 10d4abb into openSUSE:master Feb 15, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jberry-suse jberry-suse deleted the jberry-suse:update_status_comments-dashboard branch Mar 8, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.