Add pushid() and pushrev() revsets

RESOLVED FIXED

Status

Developer Services
Mercurial: Pushlog
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gps, Assigned: gps)

Tracking

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

Comment hidden (empty)
Comment hidden (mozreview-request)

Comment 2

2 years ago
mozreview-review
Comment on attachment 8781747 [details]
pushlog: add pushid(N) and pushrev(set) revsets (bug 1295780);

https://reviewboard.mozilla.org/r/72092/#review69698

::: hgext/pushlog/__init__.py:418
(Diff revision 1)
> +        """
> +        with self.conn(readonly=True) as c:
> +            if not c:
> +                return None
> +
> +            res = c.execute('SELECT id FROM pushlog WHERE id=?', (pushid,))

Why do two queries in the most common case where the id is valid, when the second query will return an empty list for the less common case where the id is not valid?

This is also largely the same code as in pushfromchangeset, and should be shared.

::: hgext/pushlog/tests/test-revset.t:132
(Diff revision 1)
> +  hg: parse error: missing argument
> +  [255]
> +
> +pushrev() returns values for single revision
> +
> +  $ hg log -r 'pushrev(0)' -T '{rev}\n'

tests with sha1s instead of numbers would be nice.
Attachment #8781747 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 5

2 years ago
mozreview-review
Comment on attachment 8782072 [details]
pushlog: use decorators to register revset predicates;

https://reviewboard.mozilla.org/r/72336/#review71324
Attachment #8782072 - Flags: review?(mh+mozilla) → review+

Comment 6

2 years ago
mozreview-review
Comment on attachment 8781747 [details]
pushlog: add pushid(N) and pushrev(set) revsets (bug 1295780);

https://reviewboard.mozilla.org/r/72092/#review71328

::: hgext/pushlog/__init__.py:399
(Diff revision 2)
>              res = c.execute('SELECT pushid from changesets WHERE node=?',
>                              (ctx.hex(),)).fetchone()
>              if not res:
>                  return None
>  
> -            pushid = res[0]
> +            return self.pushfromid(res[0])

This should probably move out of the with so that there aren't two connections created at the same time. OTOH, it might also be even better to avoid opening two connections in the first place. Not moving this out of the with and making a second call to self.conn return the existing connection would do that. Another possibility is to have pushfromid take an optional connection.
Attachment #8781747 - Flags: review?(mh+mozilla)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 10

2 years ago
mozreview-review
Comment on attachment 8784966 [details]
pushlog: extract code for obtaining a Push from a push ID;

https://reviewboard.mozilla.org/r/74294/#review72220
Attachment #8784966 - Flags: review?(mh+mozilla) → review+

Comment 11

2 years ago
mozreview-review
Comment on attachment 8781747 [details]
pushlog: add pushid(N) and pushrev(set) revsets (bug 1295780);

https://reviewboard.mozilla.org/r/72092/#review72222

::: hgext/pushlog/__init__.py:642
(Diff revision 3)
> +    with repo.pushlog.conn(readonly=True) as conn:
> +        if not conn:
> +            return revset.baseset()
> +
> +        push = repo.pushlog.pushfromid(conn, pushid)
> +        if not push:
> +            return revset.baseset()

with repo.pushlog.conn(readlonly=True) as conn:
    push = repo.pushlog.pushfromid(conn, pushid) if conn else None

if not push:
    return revset.baseset()
Attachment #8781747 - Flags: review?(mh+mozilla)

Comment 12

2 years ago
mozreview-review
Comment on attachment 8781747 [details]
pushlog: add pushid(N) and pushrev(set) revsets (bug 1295780);

https://reviewboard.mozilla.org/r/72092/#review72224
Attachment #8781747 - Flags: review+
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)

Comment 16

2 years ago
Pushed by gszorc@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/70aeab089f3c
pushlog: add pushid(N) and pushrev(set) revsets ; r=glandium
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.