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

csplit: first implementation #109

Merged
merged 1 commit into from
Nov 14, 2019
Merged

csplit: first implementation #109

merged 1 commit into from
Nov 14, 2019

Conversation

mkindahl
Copy link
Contributor

A first implementation of csplit.

@mkindahl
Copy link
Contributor Author

mkindahl commented Nov 10, 2019

Sorry about the time, not able to spend as much time as I intended. There are still things that I think can be improved in the code, but I can take them as separate PRs. Please note that the added tests need to be executed manually: there can be differences that does not indicate an issue.

@mkindahl mkindahl force-pushed the csplit branch 2 times, most recently from 15cb21c to 03a4378 Compare November 10, 2019 11:15
@mkindahl
Copy link
Contributor Author

Something seems to be wrong with the job to build using the beta compiler. Looking at the output it looks like it is stalling when retrieving the cache information:

...
creating directory /home/travis/.cargo�[0m
adding /home/travis/build/GrayJack/coreutils/target to cache�[0m
creating directory /home/travis/build/GrayJack/coreutils/target�[0m
adding /home/travis/.rustup to cache�[0m
creating directory /home/travis/.rustup�[0m
adding /home/travis/.cache/sccache to cache�[0m
creating directory /home/travis/.cache/sccache�[0m


No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#build-times-out-because-no-output-was-received

The build has been terminated

I have tried to build it locally with both nightly, stable, and beta, and it works fine.

Copy link
Owner

@GrayJack GrayJack left a comment

Choose a reason for hiding this comment

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

Awesome work, thanks!! Also don't worry about time, I gave me some days to do some other work and I returned today to this project. 😃

Besides these minor things I put here, there was some occurrences of unneeded return statement and one length compared to zero using clippy with pedantic lint allowed, can you "fix" that too?

csplit/Cargo.toml Outdated Show resolved Hide resolved
csplit/src/csplit.yml Outdated Show resolved Hide resolved
csplit/src/csplit.yml Outdated Show resolved Hide resolved
csplit/src/csplit.yml Outdated Show resolved Hide resolved
@GrayJack
Copy link
Owner

There are still things that I think can be improved in the code, but I can take them as separate PRs

Can you open issues about each one so we can easily keep track of them?

@mkindahl
Copy link
Contributor Author

There are still things that I think can be improved in the code, but I can take them as separate PRs

Can you open issues about each one so we can easily keep track of them?

Of course. :)

@GrayJack
Copy link
Owner

Looks like I pushed a something that breaks 1.37 compatibility

@GrayJack GrayJack merged commit 02f4b76 into GrayJack:master Nov 14, 2019
@mkindahl mkindahl mentioned this pull request Sep 28, 2021
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.

None yet

2 participants