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

mfs: remove sort from ListNames() #5219

Merged
merged 2 commits into from
Jul 19, 2018
Merged

mfs: remove sort from ListNames() #5219

merged 2 commits into from
Jul 19, 2018

Conversation

schomatis
Copy link
Contributor

@schomatis schomatis commented Jul 12, 2018

Fixes #5181.

License: MIT
Signed-off-by: Lucas Molas <schomatis@gmail.com>
@schomatis schomatis added status/in-progress In progress topic/files Topic files topic/technical debt Topic technical debt topic/MFS Topic MFS labels Jul 12, 2018
@schomatis schomatis self-assigned this Jul 12, 2018
@schomatis schomatis requested a review from Kubuxu as a code owner July 12, 2018 14:08
@schomatis
Copy link
Contributor Author

Checking if this upsets any test...

@schomatis
Copy link
Contributor Author

It doesn't, the sort can be added at the ipfs command level.

@schomatis
Copy link
Contributor Author

Add test.

Imitate Unix `ls` command, sort by default; disable with `-U` flag.

License: MIT
Signed-off-by: Lucas Molas <schomatis@gmail.com>
test_expect_success "unsorted listing works $EXTRA" '
ipfs files ls -U /foo > list_out &&
sort list_exp_raw > sort_list_not_exp &&
! test_cmp sort_list_not_exp list_out
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the sharness way to test that the inputs should be different?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes... Unfortunately, this'll be a bit brittle. Unfortunately, I can't think of a better way.

@schomatis schomatis added need/review Needs a review and removed status/in-progress In progress labels Jul 16, 2018
@schomatis schomatis changed the title [WIP] mfs: remove sort from ListNames() mfs: remove sort from ListNames() Jul 16, 2018
@schomatis schomatis requested a review from Stebalien July 16, 2018 13:27
Copy link
Member

@Stebalien Stebalien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@Stebalien Stebalien added RFM and removed need/review Needs a review labels Jul 18, 2018
@Stebalien
Copy link
Member

@schomatis I assume this is ready (and no longer "do not merge").

@schomatis
Copy link
Contributor Author

Yes, sorry, forgot to change that, thanks!

(I use too many anti-merge precautionary measures :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFM topic/files Topic files topic/MFS Topic MFS topic/technical debt Topic technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants