Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Flexible length for the chunk "sub identifier" #35

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Flexible length for the chunk "sub identifier" #35

wants to merge 1 commit into from

Conversation

oliverpool
Copy link

Hi,

Thank you for this great tool!

I prefer to re-assemble the file using system tools (like cat chunk_* > complete_file), but the ordering is wrong because chunk_10 is sorted before chunk_2. I would like the "sub-identifier" to be padded : chunk_02.


This PR proposes a new parameter: $pad_length.

Using this parameter, one can choose a minimum length of the "sub-identifier". I also added an optional parameter to getChunkPath, because I intend to call getChunkPath('*', 0) to get the file pattern for my cat command.

I need your help to determine the best way to set the "global" $pad_length: should I use the ConfigInterface and add a parameter there ?

@coveralls
Copy link

coveralls commented Jul 25, 2016

Coverage Status

Coverage decreased (-1.4%) to 98.63% when pulling c04a639 on oliverpool:sub-identifier-padding into b9da114 on flowjs:master.

@AidasK
Copy link
Member

AidasK commented Jul 25, 2016

It should be added to the Config class. Make sure you don't break other implementations. I think this should be released as a major version, because it can break code for others. Don't forget the tests, code coverage should be 100%

@oliverpool
Copy link
Author

It should be added to the Config class

Ok, I'll work on it.

I think this should be released as a major version, because it can break code for others.

If we set the default pad_length to 0, it shouldn't break anything.

Don't forget the tests

Sure, I'll look at it.

@AidasK
Copy link
Member

AidasK commented Jul 25, 2016

Adding a new method to a config interface is a BC. Because users might have used it in their own classes.

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.

3 participants