#51 Make non-active branches greyish and fix the calls for url_for()
Merged by pingou. Opened by pingou.
namespaced_urls  into  master

Download 51.patch
no initial comment

You also need to filter by dist-git namespace here. The type query parameter can be used for this.

It's more traditional to define functions like:
function disabled_branches() {

Is there a particular reason you are doing it this way?

It'd be clearer if the function name was disable_branches as the current name insinuates that it'd return the disabled branches.

Rename to disable_branches and will adjust function declaration

2 new commits added

  • Make the branch name grey and remove the new PR button for disabled branches
  • Add namespace to the url_for() calls

2 new commits added

  • Make the branch name grey and remove the new PR button for disabled branches
  • Add namespace to the url_for() calls

The modules namespace is also singular in PDC. The type field is a direct mapping to the values in this API:
https://pdc.fedoraproject.org/rest_api/v1/release-component-types/

Should this be in an else block? If I'm a committer on the repo, it seems that disable_branches gets run as part of the AJAX request to get_pull_request_ready_branch, and then it would get run again here too.

2 new commits added

  • Make the branch name grey and remove the new PR button for disabled branches
  • Add namespace to the url_for() calls

Should this be in an else block? If I'm a committer on the repo, it seems that disable_branches gets run as part of the AJAX request to get_pull_request_ready_branch, and then it would get run again here too.

It is run twice indeed. The first time it makes the branch show up grey the second time after the new PR button are added to remove them.

We could run them only the second time, but since it's likely faster get the info from PDC than to get the list of branches, it makes the branches appear grey sooner.

It is run twice indeed. The first time it makes the branch show up grey the second time after the new PR button are added to remove them.
We could run them only the second time, but since it's likely faster get the info from PDC than to get the list of branches, it makes the branches appear grey sooner.

Could the disable_branches function add a class (e.g. disabled_branch) to the div that contains the branch link and the new PR button? Then that class' CSS would set the PR button div to display: None. Then you could also move the color change of the disabled branch link to this CSS class as well.

That would allow you to run the query just once and it'd make the JS cleaner in my opinion.

2 new commits added

  • Make the branch name grey and remove the new PR button for disabled branches
  • Add namespace to the url_for() calls

Adjusted, it makes the 'bin' icon to delete the branch grey as well but I don't think it's an issue

Looks good to me!

Pull-Request has been merged by pingou

Metadata