Skip to content

Add a scan_and_merge API#25

Open
cgwalters wants to merge 1 commit intocoreos:mainfrom
cgwalters:merge-api
Open

Add a scan_and_merge API#25
cgwalters wants to merge 1 commit intocoreos:mainfrom
cgwalters:merge-api

Conversation

@cgwalters
Copy link
Member

Most users of this library are going to be using e.g. serde to parse the individual config files into a final merged state.

This API is similar to e.g.
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.reduce

As the API comment says - right now this is not more efficient, but it could be if we unified things internally because we could avoid allocating a BTreeMap of all the filenames.

Most users of this library are going to be using e.g. serde
to parse the individual config files into a final merged state.

This API is similar to e.g.
https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.reduce

As the API comment says - right now this is not more efficient,
but it could be if we unified things internally because we could
avoid allocating a `BTreeMap` of all the filenames.
@nabijaczleweli
Copy link
Collaborator

IDGI. How do you expect to be able to get rid of the map? The list of files to visit is only known after you visit everything.

@cgwalters
Copy link
Member Author

The list of files to visit is only known after you visit everything.

We'd likely need to invoke stat() on the possibly-overriding paths, i.e. traverse all dirs in parallel or so instead of one by one, and perform the reduce as we go.

Honestly I don't think the performance matters too much; the set of files is pretty small in most cases. The case where it may matter a bit more is when one is using a basedir of e.g. /etc instead of /etc/someapp.

This PR isn't super important to me, I just decided to try it; it worked out OK, but I wouldn't say it's super compelling.

It may be more bang-for-the-buck to just have example code that uses serde that people can copy-and paste for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants