joerg.sonnenberger added a comment.
Now blank and Python 3 clean. INLINE COMMENTS > pulkit wrote in fastexport.py:110 > These temporary variables can be prevented. Will be dropped. > pulkit wrote in fastexport.py:121 > This one is also same as `ctx.files()` I guess. I remember @martinvonz did > some cleanup here. For a merge, ctx.files() doesn't contain added or removed files relative to one parent. This is covered by tests. > pulkit wrote in fastexport.py:123 > This one seems same as `ctx.files()` It is nowadays. > pulkit wrote in fastexport.py:151 > 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. This follows the terminology used in other implementations of this functionality and being consistent on that front seems more important. That said, I'm changing it to `marks file` consistently. A single flag doesn't help with typical use cases. For incremental conversion, you only want to use the newly created marks file when the other part of the conversion (e.g. `git fast-import`) was successful too. > pulkit wrote in fastexport.py:166 > this temporary variable can be prevented too I don't like doing the lookup twice, so I would prefer to keep the variable in this case. > pulkit wrote in test-fastexport.t:117 > any reason you redirect the output in a file and then cat it instead of just > printing them on stdout? It was easier for testing. Dropping this part. 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
