-
-
Notifications
You must be signed in to change notification settings - Fork 21
Added command line interface to Clean Py #51
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
Open
kssvrk
wants to merge
3
commits into
prashantsengar:master
Choose a base branch
from
kssvrk:cli
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
Owner
|
Thanks @kssvrk ! There are a few things that I would like to change:
It should rather be like
For the point 3 in the first paragraph, I would also ask how would we move ahead when we have large files feature integrated as well. A new flag --large-files? But I think that the default behavior of cleanpy should be arranging only |
Author
|
I have a few reservations against the no default directory provided case.
Most of the time it defaults to CleanPy installation directory for
cleaning. User can not cd into any other directory and execute cleanpy from
that directory even if he uses full path of main.py because the CleanPy
uses lib.utils without any setup in environment variables.
So it wouldn't work out. Any comments?
…On Fri, 25 Dec 2020, 19:44 Prashant Sengar, ***@***.***> wrote:
Thanks @kssvrk <https://github.com/kssvrk> !
There are a few things that I would like to change:
1. If target directory is not provided, it should work on the current
directory
2. Default cleaning behaviour should be weak arrange and not strong
3. I don't think --arrange flag should be even an option since that's
the only thing that we do.
It should rather be like
1. cleanpy -> weak arrange current directory
2. cleanpy TARGET_DIR -> weak arrange TARGET_DIR
3. cleanpy --strong TARGET_DIR -> strong arrange TARGET_DIR
4. cleanpy --strong -nw -> strong arrange current directory with no
warning
For the point 3 in the first paragraph, I would also ask how would we move
ahead when we have large files feature integrated as well. A new flag
--large-files? But I think that the default behavior of cleanpy should be
arranging only
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#51 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AMCJYV2Y5Z7AMUSNMSRDQALSWSM3LANCNFSM4VI4CIJQ>
.
|
Owner
|
But since we are packaging it for PyPI, most of the users would be
installing it with pip and then use it via the command line. At that point,
it would be safe to assume that most of the time the user runs it, the
current directory is the directory where he/she wants to run it.
I hope I am getting it right.
Also this reminds me of configuration while running it via pip package. We
would need an option to let the user change configuration from the CLI. I
think I would have to do it while packaging it.
On Fri, Dec 25, 2020, 7:53 PM RADHA KRISHNA KAVULURU <
[email protected]> wrote:
… I have a few reservations against the no default directory provided case.
Most of the time it defaults to CleanPy installation directory for
cleaning. User can not cd into any other directory and execute cleanpy from
that directory even if he uses full path of main.py because the CleanPy
uses lib.utils without any setup in environment variables.
So it wouldn't work out. Any comments?
On Fri, 25 Dec 2020, 19:44 Prashant Sengar, ***@***.***>
wrote:
> Thanks @kssvrk <https://github.com/kssvrk> !
>
> There are a few things that I would like to change:
>
> 1. If target directory is not provided, it should work on the current
> directory
> 2. Default cleaning behaviour should be weak arrange and not strong
> 3. I don't think --arrange flag should be even an option since that's
> the only thing that we do.
>
> It should rather be like
>
> 1. cleanpy -> weak arrange current directory
> 2. cleanpy TARGET_DIR -> weak arrange TARGET_DIR
> 3. cleanpy --strong TARGET_DIR -> strong arrange TARGET_DIR
> 4. cleanpy --strong -nw -> strong arrange current directory with no
> warning
>
> For the point 3 in the first paragraph, I would also ask how would we
move
> ahead when we have large files feature integrated as well. A new flag
> --large-files? But I think that the default behavior of cleanpy should be
> arranging only
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <
#51 (comment)>,
> or unsubscribe
> <
https://github.com/notifications/unsubscribe-auth/AMCJYV2Y5Z7AMUSNMSRDQALSWSM3LANCNFSM4VI4CIJQ
>
> .
>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#51 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AK43YGEEWVTXUHI4NR5OQ4LSWSN4ZANCNFSM4VI4CIJQ>
.
|
Author
|
Oh yes,
So it would make sense then. So I'd have to take the current working
directory.
Ok I'd make the changes and inform.
It would look good as you said once you merge your PR of packaging.
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a PR against Issue 46
I have modified the sys argv methods and added CLI through 3 different arguments
Accordingly i modified the main file with choice variable and validation using exceptions. Did a little bit of code cleaning.