#4175 Make it possible to configure the default branch shown in the UI
Closed 3 years ago by pingou. Opened 5 years ago by pingou.

file modified
+15
@@ -1659,6 +1659,21 @@ 

  Defaults to: ``{".spec": "specfile", ".patch": "diff"}``

  

  

+ DEFAULT_UI_BRANCH

+ ~~~~~~~~~~~~~~~~~

+ 

+ This configuration key allows to change the branch shown by default in the

+ UI when accessing the overview page of the project.

+ When accessing the overview page of the project, we check if there is a

+ branch with the same name as the one specified in this configuration key

+ and if so, this is the branch shown. All links from there will be to that

+ branch, letting the user change back to another branch using either the

+ page listing all branches or the drop-down menu at the top of the page

+ listing the commits.

+ 

+ Defaults to: ``None``.

+ 

+ 

  RepoSpanner Options

  -------------------

  

file modified
+14 -11
@@ -117,34 +117,37 @@ 

      repo_db = flask.g.repo

      repo_obj = flask.g.repo_obj

  

-     if not repo_obj.is_empty and not repo_obj.head_is_unborn:

-         head = repo_obj.head.shorthand

+     default_branch = pagure_config.get('DEFAULT_UI_BRANCH')

+ 

+     if default_branch in repo_obj.listall_branches():

+         head = repo_obj.lookup_branch(default_branch).get_object().oid.hex

+         branchname = default_branch

      else:

-         head = None

+         if not repo_obj.is_empty and not repo_obj.head_is_unborn:

+             head = repo_obj.head.target

+             branchname = repo_obj.head.shorthand

+         else:

+             head = None

+             branchname = None

  

      cnt = 0

      last_commits = []

      tree = []

      if not repo_obj.is_empty:

          try:

-             for commit in repo_obj.walk(

-                 repo_obj.head.target, pygit2.GIT_SORT_NONE

-             ):

+             for commit in repo_obj.walk(head, pygit2.GIT_SORT_NONE):

                  last_commits.append(commit)

                  cnt += 1

                  if cnt == 3:

                      break

-             tree = sorted(last_commits[0].tree, key=lambda x: x.filemode)

+             if last_commits:

+                 tree = sorted(last_commits[0].tree, key=lambda x: x.filemode)

          except pygit2.GitError:

              pass

  

      readme = None

      safe = False

  

-     if not repo_obj.is_empty and not repo_obj.head_is_unborn:

-         branchname = repo_obj.head.shorthand

-     else:

-         branchname = None

      project = pagure.lib.query.get_authorized_project(

          flask.g.session, repo, user=username, namespace=namespace

      )

@@ -1822,6 +1822,75 @@ 

          self.perfMaxWalks(3, 18)  # Ideal: (1, 3)

          self.perfReset()

  

+     @patch.dict('pagure.config.config', {'DEFAULT_UI_BRANCH': 'feature'})

+     def test_view_repo_default_branch_feature(self):

+         """ Test the view_repo endpoint when a default branch is specified.

+         """

+ 

+         tests.create_projects(self.session)

+         tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True)

+ 

+         # Add some content to the git repo

+         tests.add_content_git_repo(

+             os.path.join(self.path, 'repos', 'test.git'))

+         tests.add_readme_git_repo(

+             os.path.join(self.path, 'repos', 'test.git'), branch="feature")

+         self.perfReset()

+ 

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

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         self.assertNotIn('<p>This repo is brand new!</p>', output_text)

+         self.assertIn(

+             '<title>Overview - test - Pagure</title>', output_text)

+         self.assertIn(

+             '<span class="font-weight-bold">Add a README file</span>',

+             output_text)

+         self.assertNotIn(

+             '<span class="font-weight-bold">Add some directory and a file '

+             'for more testing</span>', output_text)

+         self.assertIn(

+             'href="/test/branches?branchname=feature">', output_text)

+         self.assertNotIn(

+             'href="/test/branches?branchname=master">', output_text)

+         self.perfMaxWalks(3, 8)  # Target: (1, 3)

+         self.perfReset()

+ 

+     @patch.dict('pagure.config.config', {'DEFAULT_UI_BRANCH': 'foobar'})

+     def test_view_repo_default_branch_invalid(self):

+         """ Test the view_repo endpoint when an invalid default branch is

+         specified (in which case it goes back to the default branch).

+         """

+ 

+         tests.create_projects(self.session)

+         tests.create_projects_git(os.path.join(self.path, 'repos'), bare=True)

+ 

+         # Add some content to the git repo

+         tests.add_content_git_repo(

+             os.path.join(self.path, 'repos', 'test.git'))

+         tests.add_readme_git_repo(

+             os.path.join(self.path, 'repos', 'test.git'), branch="feature")

+         self.perfReset()

+ 

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

+         self.assertEqual(output.status_code, 200)

+         output_text = output.get_data(as_text=True)

+         self.assertNotIn('<p>This repo is brand new!</p>', output_text)

+         self.assertIn(

+             '<title>Overview - test - Pagure</title>', output_text)

+         self.assertIn(

+             '<span class="font-weight-bold">Add some directory and a file '

+             'for more testing</span>', output_text)

+         self.assertNotIn(

+             '<span class="font-weight-bold">Add a README file</span>',

+             output_text)

+         self.assertIn(

+             'href="/test/branches?branchname=master">', output_text)

+         self.assertNotIn(

+             'href="/test/branches?branchname=feature">', output_text)

+         self.perfMaxWalks(3, 8)  # Target: (1, 3)

+         self.perfReset()

+ 

      def test_view_repo_empty(self):

          """ Test the view_repo endpoint on a repo w/o master branch. """

  

I still think this is a bad idea to implement. This will add confusion since it doesn't change the actual default branch in the Git repo itself.

since it doesn't change the actual default branch in the Git repo itself

That's in purpose.

The idea/issue is to display the epel7 branch by default on CentOS' dist-git while keeping master in Fedora with both instances being in sync. If we change the default git branch, it will be synced from one instance to the other, thus making either CentOS' default branch be master, or Fedora's be epel7.

@jperrin do you want to shim in about the comment on added confusion?

I know the purpose, I'm just saying it's a bad idea to change the default view this way, because it is misleading to the poor folks who check out the code from the CentOS Dist-Git server.

rebased onto 57899d6c3cc69dee30404bd270ff2e4c76c767a1

5 years ago

rebased onto e4e2750

5 years ago

We never heard back from the stakeholder asking for this feature, so I'm going to close this PR and its associated ticket.

Pull-Request has been closed by pingou

3 years ago

@pingou Just for future reference, in case this is re-opened: this change also needs to account for all the other UI quirks that break when you have a default ref that doesn't exist (new PR creation, commits view, etc.).