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

Link to backend info from project edit form #556

Merged
merged 3 commits into from Jan 8, 2019
Merged

Link to backend info from project edit form #556

merged 3 commits into from Jan 8, 2019

Conversation

abitrolly
Copy link
Contributor

No description provided.

@codecov-io
Copy link

codecov-io commented May 15, 2018

Codecov Report

Merging #556 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #556   +/-   ##
=======================================
  Coverage   96.51%   96.51%           
=======================================
  Files          57       57           
  Lines        2985     2985           
  Branches      407      407           
=======================================
  Hits         2881     2881           
  Misses         70       70           
  Partials       34       34

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d947fce...8bd834d. Read the comment docs.

@nqb
Copy link
Contributor

nqb commented May 15, 2018

Not agree with this commit.

<a href="https://release-monitoring.org/about#backends"> : will work only for release-monitoring.org instance.

Please use relative path.

@abitrolly
Copy link
Contributor Author

You mean absolute to the root of the web-site? Because I don't remember URL for this form to calculate relative path.

If the path is relative to web-site root, then can you point me to documentation to find the helper for this site root, because I am not familiar with this framework?

@abitrolly
Copy link
Contributor Author

@nqb I just copy/pasted what seems to be logical for me. Should work.

@nqb
Copy link
Contributor

nqb commented May 15, 2018

Hello @abitrolly,

Seems better but I can't test. @jeremycline, your opinion ?

@jeremycline
Copy link
Member

Hi @abitrolly,

Thanks for the PR. I'm on vacation at the moment, but I'll take a look at this when I get back next week. The code itself looks reasonable. Does this fix an open issue on the issue tracker?

@abitrolly
Copy link
Contributor Author

Does this fix an open issue on the issue tracker?

No, only self.

@abitrolly
Copy link
Contributor Author

@jeremycline so did you get the time to review this?

@jeremycline
Copy link
Member

Sorry, it dropped off my radar. I just gave this a test, it looks like it's not accounting for the documentation having moved away from /about. The new href would be href="{{ url_for('anitya_ui.static', filename='docs/user-guide.html') }}#backends", but perhaps it would be better to just link to the user guide as a whole from the new project page (in other words, use the #creating-a-new-project fragment and put it at the top somewhere as "Help" or something). Having just the backends linked is a bit odd when there's a whole guide.

@abitrolly
Copy link
Contributor Author

The link is for helping users quickly choose proper backend. User guide is already available from the top menu.

@jeremycline
Copy link
Member

What I'm suggesting is that if someone doesn't know what backend they want, they probably want all the documentation on creating a new project. The documentation generally is linked at the top, but linking directly to the "create a project" documentation is more helpful. If just the backend field is linked, I (as a user) will wonder why the rest of the fields aren't linked or don't have documentation.

The adjustment you've made doesn't look like it'll render, however. You've got }}#backends }}#backends

@abitrolly
Copy link
Contributor Author

abitrolly commented Jul 24, 2018

I fixed the link, but until 0.12 is released #561 there it not much motivation to add other documentation links. I don't like layout of Sphinx docs and they are hard to customize, so nobody does it.

As for if users should read the full doc or not - it's up to them - the link to the full doc is in the header. I was scratching my own itch.

@Zlopez
Copy link
Contributor

Zlopez commented Oct 11, 2018

Now when there is version 0.13.1 live I want to add this PR.

Just one note, this change is made in project.html template, but according to what you wrote, you want this link to be visible when the user is creating new project.
So you should do the change in project_new.html template, but I left this up to you.

@abitrolly
Copy link
Contributor Author

Rebased. Making a link for edit/new form is doable. I can do this right away. Without testing as I am not sure I have time to setup testing environment. https://github.com/release-monitoring/anitya/blob/master/anitya/templates/functions.html

@abitrolly
Copy link
Contributor Author

I added the code that should add the description to new/edit project form. It is completely theoretical and untested, because I can not get past login screen in development environment.

#689 (comment)

@Zlopez
Copy link
Contributor

Zlopez commented Jan 7, 2019

To test it in current state, you need to delete check_user event in anitya/db/events.py.
This will be fixed by #669

@abitrolly
Copy link
Contributor Author

Just commenting decorator helped. PR works ok.

@abitrolly
Copy link
Contributor Author

Squashed commits.

@Zlopez
Copy link
Contributor

Zlopez commented Jan 8, 2019

Looks good. Could you just add news file for townscryer? Just add a new file to news folder.

@mergify mergify bot merged commit 7081ca6 into fedora-infra:master Jan 8, 2019
@abitrolly
Copy link
Contributor Author

It is possible to autogenerate links for adding towncrier files. Like https://github.com/abitrolly/anitya/new/patch-1?filename=news/556.feature

@abitrolly abitrolly deleted the patch-1 branch January 8, 2019 11:04
@Zlopez
Copy link
Contributor

Zlopez commented Jan 8, 2019

Link isn't working

@abitrolly
Copy link
Contributor Author

Because I removed my branch.

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

5 participants