Edgewall Software
Modify

Opened 7 years ago

Closed 7 years ago

#12182 closed enhancement (fixed)

short_rev and display_rev should always return a string

Reported by: Ryan J Ollos Owned by: Ryan J Ollos
Priority: normal Milestone: 1.2
Component: version control Version:
Severity: normal Keywords:
Cc: Branch:
Release Notes:

Repository.short_rev and Repository.display_rev always return a string or None.

API Changes:
Internal Changes:

Description (last modified by Ryan J Ollos)

Discussed in #12179, the API should document that short_rev and display_rev return a string, and the Subversion implementation should be modified to return a string rather than an integer. Since short_rev and display_rev call normalize_rev, it may be that the latter only needs to be modified.

Attachments (1)

t12182.diff (3.7 KB ) - added by Ryan J Ollos 7 years ago.

Download all attachments as: .zip

Change History (5)

comment:1 by Ryan J Ollos, 7 years ago

Owner: set to Ryan J Ollos
Status: newassigned

by Ryan J Ollos, 7 years ago

Attachment: t12182.diff added

comment:2 by Ryan J Ollos, 7 years ago

Proposed change in attachment:t12182.diff.

comment:3 by Jun Omae, 7 years ago

I tried the patch and got 4 failures.

FAIL: test_repos_display_rev (tracopt.versioncontrol.svn.tests.svn_fs.SubversionRepositoryNormalTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "tracopt/versioncontrol/svn/tests/svn_fs.py", line 140, in test_repos_display_rev
    self.assertEqual(str(HEAD), self.repos.display_rev(None))
AssertionError: '30' != 30

Probably, the following would solve it.

  • trac/versioncontrol/api.py

    diff --git a/trac/versioncontrol/api.py b/trac/versioncontrol/api.py
    index b22bca4be..62a3cc7ec 100644
    a b class Repository(object):  
    10191019
    10201020        :raise NoSuchChangeset: If the given `rev` isn't found.
    10211021        """
    1022         norm_rev = self.normalize_rev(rev)
    1023         return str(norm_rev) if rev is not None else norm_rev
     1022        rev = self.normalize_rev(rev)
     1023        return str(rev) if rev is not None else rev
    10241024
    10251025    def display_rev(self, rev):
    10261026        """Return a string representation of a revision in the repos for
    class Repository(object):  
    10311031
    10321032        :raise NoSuchChangeset: If the given `rev` isn't found.
    10331033        """
    1034         norm_rev = self.normalize_rev(rev)
    1035         return str(norm_rev) if rev is not None else norm_rev
     1034        rev = self.normalize_rev(rev)
     1035        return str(rev) if rev is not None else rev
    10361036
    10371037    @abstractmethod
    10381038    def get_changes(self, old_path, old_rev, new_path, new_rev,

comment:4 by Ryan J Ollos, 7 years ago

Description: modified (diff)
Release Notes: modified (diff)
Resolution: fixed
Status: assignedclosed

Thanks for review. Committed to trunk in [14273].

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Ryan J Ollos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ryan J Ollos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.