#2144 Minor text fixes
Merged 7 years ago by slaznick@redhat.com. Opened 7 years ago by stlaz.
stlaz/pagure text_fixes  into  master

file modified
+2 -2
@@ -207,7 +207,7 @@ 

  

  

  class RequestPullForm(PagureForm):

-     ''' Form to create a request pull. '''

+     ''' Form to create a pull request. '''

      title = wtforms.TextField(

          'Title<span class="error">*</span>',

          [wtforms.validators.Required()]
@@ -217,7 +217,7 @@ 

  

  

  class RemoteRequestPullForm(RequestPullForm):

-     ''' Form to create a remote request pull. '''

+     ''' Form to create a remote pull request. '''

      git_repo = wtforms.TextField(

          'Git repo address<span class="error">*</span>',

          [wtforms.validators.Required()]

file modified
+2 -1
@@ -1356,7 +1356,8 @@ 

  

      if repo_obj.is_empty:

          raise pagure.exceptions.PagureException(

-             'Fork is empty, there are no commits to request pulling')

+             'Fork is empty, there are no commits to create a pull request with'

+         )

  

      if not orig_repo.is_empty \

              and request.branch not in orig_repo.listall_branches():

@@ -84,7 +84,7 @@ 

                      username=username,

                      namespace=repo.namespace,

                      branch_to=head, branch_from=branchname or 'master') }}">

-               {% if g.repo_committer %}Request pull{% else %}Compare{% endif %}

+               {% if g.repo_committer %}Create pull request{% else %}Compare{% endif %}

              </a>

            </div>

        {% endif %}

file modified
+2 -2
@@ -55,14 +55,14 @@ 

        (100.0 * (oth_issues_cnt / total_issues_cnt))|round|int }}% of {%

        if status|lower in ['open', 'true'] %}closed{%

        else %}open{%

-       endif %} issues on {{ total_issues_cnt }} issues in total">

+       endif %} issues of total {{ total_issues_cnt }} issues">

        {% if (issues | length + oth_issues_cnt) %}

          <span style="width: {{

            (100.0 * (issues_cnt / total_issues_cnt))|round|int }}%" title="{{

            (100.0 * (issues_cnt / total_issues_cnt))|round|int }}% of {%

            if status|lower in ['open', 'true'] %}open{%

            else %}closed{%

-           endif %} issues on {{ total_issues_cnt }} issues in total">

+           endif %} issues of total {{ total_issues_cnt }} issues">

              {{ (100.0 * (issues_cnt / total_issues_cnt))|round|int }}%

          </span>

        {% endif %}

@@ -316,7 +316,7 @@ 

                  <span class="oi" data-glyph="random"> </span> \

                  {{ head }}</span> \

                  <div id="request_pull" class="col-md-2"> \

-                 <a class="btn btn-primary btn-sm" href="' + url + branch + '"> Request pull </a> \

+                 <a class="btn btn-primary btn-sm" href="' + url + branch + '"> Create pull request </a> \

                  </div></div>';

            /*$($('.bodycontent').find('.row').children()[0]).before(html);*/

            {% if repo.is_fork %}

file modified
+10 -9
@@ -122,7 +122,7 @@ 

          diff = repo_commit.tree.diff_to_tree(swap=True)

      else:

          raise pagure.exceptions.PagureException(

-             'Fork is empty, there are no commits to request pulling'

+             'Fork is empty, there are no commits to create a pull request with'

          )

  

      return(diff, diff_commits, orig_commit)
@@ -137,7 +137,7 @@ 

  @APP.route('/fork/<username>/<namespace>/<repo>/pull-requests/')

  @APP.route('/fork/<username>/<namespace>/<repo>/pull-requests')

  def request_pulls(repo, username=None, namespace=None):

-     """ Request pulling the changes from the fork into the project.

+     """ Create a pull request with the changes from the fork into the project.

      """

      status = flask.request.args.get('status', 'Open')

      assignee = flask.request.args.get('assignee', None)
@@ -238,7 +238,7 @@ 

  @APP.route(

      '/fork/<username>/<namespace>/<repo>/pull-request/<int:requestid>')

  def request_pull(repo, requestid, username=None, namespace=None):

-     """ Request pulling the changes from the fork into the project.

+     """ Create a pull request with the changes from the fork into the project.

      """

  

      repo = flask.g.repo
@@ -443,7 +443,7 @@ 

          SESSION.add(request)

          try:

              SESSION.commit()

-             flask.flash('Request pull edited!')

+             flask.flash('Pull request edited!')

          except SQLAlchemyError as err:  # pragma: no cover

              SESSION.rollback()

              APP.logger.exception(err)
@@ -749,7 +749,7 @@ 

      methods=['POST'])

  @login_required

  def merge_request_pull(repo, requestid, username=None, namespace=None):

-     """ Request pulling the changes from the fork into the project.

+     """ Create a pull request with the changes from the fork into the project.

      """

  

      form = pagure.forms.ConfirmationForm()
@@ -828,7 +828,7 @@ 

      methods=['POST'])

  @login_required

  def cancel_request_pull(repo, requestid, username=None, namespace=None):

-     """ Cancel request pulling request.

+     """ Cancel a pull request.

      """

  

      form = pagure.forms.ConfirmationForm()
@@ -855,7 +855,7 @@ 

              merged=False)

          try:

              SESSION.commit()

-             flask.flash('Request pull canceled!')

+             flask.flash('Pull request canceled!')

          except SQLAlchemyError as err:  # pragma: no cover

              SESSION.rollback()

              APP.logger.exception(err)
@@ -1016,7 +1016,7 @@ 

      '<path:branch_to>..<path:branch_from>', methods=('GET', 'POST'))

  def new_request_pull(

          repo, branch_to, branch_from, username=None, namespace=None):

-     """ Request pulling the changes from the fork into the project.

+     """ Create a pull request with the changes from the fork into the project.

      """

      branch_to = flask.request.values.get('branch_to', branch_to)

  
@@ -1170,7 +1170,8 @@ 

      methods=('GET', 'POST'))

  @login_required

  def new_remote_request_pull(repo, username=None, namespace=None):

-     """ Request pulling the changes from a remote fork into the project.

+     """ Create a pull request with the changes from a remote fork into the

+         project.

      """

      confirm = flask.request.values.get('confirm', False)

  

@@ -717,7 +717,7 @@ 

              '<title>Overview - test - Pagure</title>', output.data)

          self.assertIn(

              '</button>\n                      Fork is empty, there are no '

-             'commits to request pulling', output.data)

+             'commits to create a pull request with', output.data)

  

          shutil.rmtree(newpath)

  
@@ -1089,7 +1089,7 @@ 

              '<title>Overview - test - Pagure</title>', output.data)

          self.assertIn(

              '</button>\n                      Fork is empty, there are no '

-             'commits to request pulling', output.data)

+             'commits to create a pull request with', output.data)

  

          shutil.rmtree(newpath)

  
@@ -1171,7 +1171,7 @@ 

              self.assertIn(

                  '<title>Overview - test - Pagure</title>', output.data)

              self.assertIn(

-                 '</button>\n                      Request pull canceled!',

+                 '</button>\n                      Pull request canceled!',

                  output.data)

  

      @patch('pagure.lib.notify.send_email')
@@ -1536,7 +1536,7 @@ 

                  '<title>Overview - test - Pagure</title>', output.data)

              self.assertIn(

                  '</button>\n                      Fork is empty, there are '

-                 'no commits to request pulling', output.data)

+                 'no commits to create a pull request with', output.data)

  

              output = self.app.get('/test/new_issue')

              csrf_token = output.data.split(
@@ -1554,7 +1554,7 @@ 

                  '<title>Overview - test - Pagure</title>', output.data)

              self.assertIn(

                  '</button>\n                      Fork is empty, there are '

-                 'no commits to request pulling', output.data)

+                 'no commits to create a pull request with', output.data)

  

          shutil.rmtree(newpath)

  
@@ -1590,7 +1590,7 @@ 

                  '<title>Overview - test - Pagure</title>', output.data)

              self.assertIn(

                  '</button>\n                      Fork is empty, there are '

-                 'no commits to request pulling', output.data)

+                 'no commits to create a pull request with', output.data)

  

          shutil.rmtree(newpath)

  

In issues display:
- A typo, substitute "on" with "of"
It's called a "pull request" and people are used to it, don't try
to be smarter than the industry. Also, be consistent of how you call
it across the whole system.

rebased

7 years ago

don't try to be smarter than the industry

So nice...

This PR makes it incosistent :)

pagure/ui/fork.py:125:            'Fork is empty, there are no commits to request pulling'
pagure/ui/fork.py:446:            flask.flash('Request pull edited!')
pagure/ui/fork.py:858:            flask.flash('Request pull canceled!')

@pingou:
I might have overreacted when I saw the nightmare of "Request pull". However, I heard about you calling "pull requests" this way in your api as well and could not believe until I saw.
I suggest you rename it at all spots in the system to something people are used to till you can (backward compatibility will be a big pain once you've reached stable), this can only create frustrations such as mine in the future as people will be confused with what you are doing there by just calling it differently.
If not anything else, please, think about how bad/rude "Request pull" sounds. It reads much more like something you would request from your girl/boyfriend.

Well, have you tried running git request-pull --help in a shell?

Just as an extra data point: Gitlab calls the same thing Merge request.

Though I do think we should be consistent and that's likely mean going with pull-request since that's what we use in our URLs, though I kinda like the idea of being closer to git itself.

@pingou I believe the people behind GitHub know that "requesting a pull" sounds nasty and that's exactly the reason why they are not calling it that. "Merge request" is also fine as far as I am concerned.
I haven't seen anyone ever use git request-pull although that of course does not mean people are not using it, that just means I haven't seen anyone using it.
Rebased.

rebased

7 years ago

If we're going with it, we should address the point raised by @mbasti above (and likely check if the tests are affected)

It affects tests and also docstrings

rebased

7 years ago

The test issues should be resolved now, sorry it took me such a long time, had to do some of my work first. Also rebased on current master so that there are no merge issues.
I too reworded the commit message to something more sane, hope you'll like it better :)

rebased

7 years ago

Looking good!

I'm going to merge this manually to save you the rebase :)

Pull-Request has been merged by slaznick@redhat.com

7 years ago