Page MenuHomePhabricator

util: extract all date-related utils in utils/dateutil module
ClosedPublic

Authored by lothiraldan on Feb 15 2018, 11:28 AM.

Details

Summary

With this commit, util.py lose 262 lines

Note for extensions author, if this commit breaks your extension, you can pull
the step-by-step split here to help you more easily pinpoint the renaming that
broke your extension:

hg pull https://bitbucket.org/octobus/mercurial-devel/ -r ac1f6453010d

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

lothiraldan created this revision.Feb 15 2018, 11:28 AM
martinvonz added inline comments.
mercurial/utils/dateutil.py
15–20

What else do you foresee moving into the util/ directory? Perhaps these?

dlax added a subscriber: dlax.Feb 16 2018, 2:59 AM

Having both a util module and a utils package looks weird.
Have you considered moving util.py into util/__init__.py and then adding new modules under util package?

In D2282#37751, @dlax wrote:

Having both a util module and a utils package looks weird.
Have you considered moving util.py into util/__init__.py and then adding new modules under util package?

I have but I'm always very cautious when creating a new package with the name of an old module. .pyc/.pycache files may still be there both for Mercurial developers and for Mercurial users using their deb/rpm package.

I may be wrong, but if we could avoid weird bugs, I would prefer put in the utils package.

I could have also copy util.py as utils/__init__.py but then I would have to update basically all Mercurial Python source files.

mercurial/utils/dateutil.py
15–20

I'm thinking maybe:

  • All net related functions in netutil.py
  • All paths related functions in pathutil.py
  • All cache related functions in cacheutil.py
dlax added a comment.Feb 16 2018, 4:41 AM

lothiraldan (Boris Feld) wrote:

I have but I'm always very cautious when creating a new package with the
name of an old module. .pyc/.pycache files may still be there both for
Mercurial developers and for Mercurial users using their deb/rpm package.
I may be wrong, but if we could avoid weird bugs, I would prefer put in
the utils package.

Well, this is Python... Most people are aware of this kind of issues, I
think. So the argument sounds a bit like FUD :)

On the other hand, if we keep going that route, we'll have to live with
util/utils "forever" just to have avoided this hypothetical
inconvenience, is this more acceptable?

In D2282#37757, @dlax wrote:

lothiraldan (Boris Feld) wrote:

I have but I'm always very cautious when creating a new package with the
name of an old module. .pyc/.pycache files may still be there both for
Mercurial developers and for Mercurial users using their deb/rpm package.
I may be wrong, but if we could avoid weird bugs, I would prefer put in
the utils package.

Well, this is Python... Most people are aware of this kind of issues, I
think. So the argument sounds a bit like FUD :)
On the other hand, if we keep going that route, we'll have to live with
util/utils "forever" just to have avoided this hypothetical
inconvenience, is this more acceptable?

I would love to be wrong on this, just remember that Mercurial users are not necessarily Python developers.

@martinvonz @durin42 what d you think about copying util.py as util/__init__.py?

In D2282#37757, @dlax wrote:

lothiraldan (Boris Feld) wrote:

I have but I'm always very cautious when creating a new package with the
name of an old module. .pyc/.pycache files may still be there both for
Mercurial developers and for Mercurial users using their deb/rpm package.
I may be wrong, but if we could avoid weird bugs, I would prefer put in
the utils package.

Well, this is Python... Most people are aware of this kind of issues, I
think. So the argument sounds a bit like FUD :)
On the other hand, if we keep going that route, we'll have to live with
util/utils "forever" just to have avoided this hypothetical
inconvenience, is this more acceptable?

I would love to be wrong on this, just remember that Mercurial users are not necessarily Python developers.
@martinvonz @durin42 what d you think about copying util.py as util/__init__.py?

I don't know much about Python. Is it defined whether util.pyc or util/__init__.py will take precedence, and, if so, which is it?

In D2282#37757, @dlax wrote:

lothiraldan (Boris Feld) wrote:

I have but I'm always very cautious when creating a new package with the
name of an old module. .pyc/.pycache files may still be there both for
Mercurial developers and for Mercurial users using their deb/rpm package.
I may be wrong, but if we could avoid weird bugs, I would prefer put in
the utils package.

Well, this is Python... Most people are aware of this kind of issues, I
think. So the argument sounds a bit like FUD :)
On the other hand, if we keep going that route, we'll have to live with
util/utils "forever" just to have avoided this hypothetical
inconvenience, is this more acceptable?

I would love to be wrong on this, just remember that Mercurial users are not necessarily Python developers.
@martinvonz @durin42 what d you think about copying util.py as util/__init__.py?

I don't know much about Python. Is it defined whether util.pyc or util/__init__.py will take precedence, and, if so, which is it?

I asked Alex Gaynor about this and it's not an immediate answer ("You get a basket of trouble. What python version?")

Could we make the package version be utils (note trailing ess) and migrate things to the package as we have time, making util be the legacy forwarding name and nothing more?

I asked Alex Gaynor about this and it's not an immediate answer ("You get a basket of trouble. What python version?")
Could we make the package version be utils (note trailing ess) and migrate things to the package as we have time, making util be the legacy forwarding name and nothing more?

That's the current proposal

durin42 accepted this revision as: durin42.Mar 2 2018, 1:23 PM

I'm fine with this but would like someone else to ack it.

I'm fine with this but would like someone else to ack it.

Ack

I'm happy to land this, but it's going to need rebased.

lothiraldan updated this revision to Diff 6400.Mar 2 2018, 6:14 PM
durin42 accepted this revision.Mar 2 2018, 6:37 PM
This revision is now accepted and ready to land.Mar 2 2018, 6:37 PM
This revision was automatically updated to reflect the committed changes.