Page MenuHomePhabricator

hg-core: add configparser to library
Needs RevisionPublic

Authored by indygreg on Dec 7 2019, 3:17 PM.


Group Reviewers

This commit contains all the changes to the recently-imported
Rust code to make it work with our Rust library and pass our
linting tests.

We rename to to reflect it being a sub-module instead
of a full library. We add this module to our We add various
package dependencies to Cargo.toml. We make a handful of Rust code
updates to reflect the new symbol locations. Finally, we add a containing the boilerplate for generating a config file
parser from the spec.pest file.

As part of adding the dependencies, some versions changed slightly
from the upstream Cargo.toml, as we already had referenced a more
modern crate version.

Diff Detail

rHG Mercurial
No Linters Available
No Unit Test Coverage

Event Timeline

indygreg created this revision.Dec 7 2019, 3:17 PM
Alphare added a subscriber: Alphare.Dec 9 2019, 8:00 AM
Alphare added inline comments.

"The current minimum required Rust version is 1.36". This disqualifies this crate from being used in hg-core right now. Good news is that it is supposed to offer improvements over existing std types like Mutex and the like, so it's nothing that would be hard to replace.

Also, I've seen complaints online about the lack of benchmarks of parking_lot against std types to back-up their claims for performance. I have not looked into it myself.

Alphare added inline comments.Dec 12 2019, 3:48 AM

I wonder about this dependency. It uses 4 usize for every slice of bytes, and adds some overhead, albeit minimal, that I am trying to justify.
Are we looking at reaping the benefits down the line? The crate itself is good quality, maintained by the Tokio people, so I'm not *too* worried about it being pre-1.0, but do we need this?

Alphare added inline comments.Dec 12 2019, 2:57 PM

I know it doesn't matter at all, but the timing of this issue is just too funny not to share:

I'm a little lost on the state of this. It looks like we need to downgrade the required Rust version?

@Alphare What's your current opinion on this diff, can you mark it accepted or request change appropriately ?

baymax requested changes to this revision.Jul 15 2020, 12:20 PM

There seems to have been no activities on this Diff for the past 3 Months.

By policy, we are automatically moving it out of the need-review state.

Please, move it back to need-review without hesitation if this diff should still be discussed.


This revision now requires changes to proceed.Jul 15 2020, 12:20 PM