pulkit added a comment.
pulkit added subscribers: martinvonz, pulkit.
The logic looks fine, the code needs to be polished a bit and need some
documentation. I left inline comments/nits. It will be nice to have the next
version of this patch py3 compatible.
INLINE COMMENTS
> fastexport.py:5
> +# GNU General Public License version 2 or any later version.
> +'''export repositories as git fast-import stream'''
> +from __future__ import absolute_import
TBH I know pretty less about git fast-import, so this help needs more
description.
> fastexport.py:105
> +
> + mark = len(marks) + 1
> + marks[revid] = mark
it's hard to follow why this is done, need a comment
> fastexport.py:110
> + ref = convert_to_git_ref(ctx.branch())
> + description = ctx.description()
> + buf = ['commit %s\n' % ref,
These temporary variables can be prevented.
> fastexport.py:121
> + p0ctx = repo[parents[0]]
> + files = ctx.manifest().diff(p0ctx.manifest())
> + else:
This one is also same as `ctx.files()` I guess. I remember @martinvonz did
some cleanup here.
> fastexport.py:123
> + else:
> + files = ctx.repo().changelog.readfiles(ctx.node())
> + filebuf = []
This one seems same as `ctx.files()`
> fastexport.py:151
> + ('e', 'export-marks', '',
> + _('new marker file to write'), _('FILE')),
> + ('A', 'authormap', '',
s/marks/marker seems clearer in the flag name.
It's not clear what a marker means. There seems to be no tests for these flags
too.
Also, what do you think about having a single flag where you read markers from
that file and write back to it.
> fastexport.py:156
> + helpcategory=command.CATEGORY_IMPORT_EXPORT)
> +def fastexport(ui, repo, *revs, **opts):
> + opts = pycompat.byteskwargs(opts)
This function needs some documentation love.
> fastexport.py:166
> + raise error.Abort(_('no revisions matched'))
> + authorfile = opts.get('authormap')
> + if authorfile:
this temporary variable can be prevented too
> test-fastexport.t:117
> +
> + $ hg fastexport > fastexport.blob
> + $ cat fastexport.blob
any reason you redirect the output in a file and then cat it instead of just
printing them on stdout?
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST ACTION
https://phab.mercurial-scm.org/D7733/new/
REVISION DETAIL
https://phab.mercurial-scm.org/D7733
To: joerg.sonnenberger, #hg-reviewers, durin42
Cc: pulkit, martinvonz, durin42, mjpieters, mercurial-devel
_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
