ls: Refactored options and other long constants to fix formatting#2436
Merged
sylvestre merged 1 commit intouutils:masterfrom Jul 4, 2021
Merged
Conversation
sylvestre
reviewed
Jun 19, 2021
sylvestre
reviewed
Jun 19, 2021
Contributor
Author
|
Any idea how do I see the full report for this run here? https://github.com/uutils/coreutils/pull/2436/checks?check_run_id=2864586345 I can't expand the error ones. @sylvestre |
Contributor
|
I can't see it either. I re-triggered the run |
Contributor
|
could you please fix the conflict? thanks |
hbina
force-pushed
the
hbina-ls-refactor-options-module
branch
from
1ca2325 to
4b5e7f9
Compare
Contributor
|
sorry but it is still conflicting |
Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Keep one of the texts in-place Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com> Reduced the fix to just formatting changes Signed-off-by: Hanif Bin Ariffin <hanif.ariffin.4326@gmail.com>
hbina
force-pushed
the
hbina-ls-refactor-options-module
branch
from
4b5e7f9 to
4778a64
Compare
Contributor
Author
|
Rebased again. |
rivy
suggested changes
Jun 27, 2021
Comment on lines
-48
to
-55
| static ABOUT: &str = " | ||
| By default, ls will list the files and contents of any directories on | ||
| the command line, expect that it will ignore files and directories | ||
| whose names start with '.' | ||
| "; | ||
| static AFTER_HELP: &str = "The TIME_STYLE argument can be full-iso, long-iso, iso. | ||
| Also the TIME_STYLE environment variable sets the default style to use."; | ||
|
|
Contributor
There was a problem hiding this comment.
Why are you moving these?
The current form follows the pattern of other utils, so it should remain unless you have some reason to move them.
Contributor
There was a problem hiding this comment.
In addition to @rivy's comment, we can consider moving those constants under our mod options but yeah please don't break the consistency for now.
pub mod options {
pub mod help {
pub static HELP: &str = "help";
}
}
Contributor
|
Technical nit... use present tense for the commit message please. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Only refactors the
optionsmodule and a bunch of long strings as opposed to full refactor here #2434Signed-off-by: Hanif Bin Ariffin hanif.ariffin.4326@gmail.com