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

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. :)

@coveralls
Copy link

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
Copy link
Contributor Author

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
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
Copy link
Contributor Author

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
Copy link
Contributor Author

@jberry-suse
Copy link
Contributor Author

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

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

@lnussel
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
@jberry-suse jberry-suse deleted the update_status_comments-dashboard branch March 8, 2017 21:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants