In wall mode, even if the wall is empty, the script goes on until the for loop reaches 5000 (with 5 sec sleep each time).
In order to be more effective, I propose to try to refresh the page n times (cli and setting parameter, default n=3), and if the delete button is not found, then, we break the for loop.
In wall mode, even if the wall is empty, the script goes on until the for loop reaches 5000 (with 5 sec sleep each time).
In order to be more effective, I propose to try to refresh the page n times (cli and setting parameter, default n=3), and if the delete button is not found, then, we break the for loop.
I rarely use env variables, but if others do I can also check for os.environ["DELETEFB_TRY_BEFORE_QUIT"] for example. It is no big deal to me. Maybe it worth considering of a user configuration file also ? ~/.config/deletefb.yml for example ?
I rarely use env variables, but if others do I can also check for os.environ["DELETEFB_TRY_BEFORE_QUIT"] for example. It is no big deal to me. Maybe it worth considering of a user configuration file also ? ~/.config/deletefb.yml for example ?
I rarely use env variables, but if others do I can also check for os.environ["DELETEFB_TRY_BEFORE_QUIT"] for example. It is no big deal to me. Maybe it worth considering of a user configuration file also ? ~/.config/deletefb.yml for example ?
Yeah I just want to avoid having too many options for things that people will rarely need to use. Environment variables are nice because you won't see it when you do --help but if you really need it you can look at the documentation and then just do FOO=bar deletefb.
Also I think it should be something like RETRY_THRESHOLD because it's shorter.
> I rarely use env variables, but if others do I can also check for os.environ["DELETEFB_TRY_BEFORE_QUIT"] for example. It is no big deal to me. Maybe it worth considering of a user configuration file also ? ~/.config/deletefb.yml for example ?
Yeah I just want to avoid having too many options for things that people will rarely need to use. Environment variables are nice because you won't see it when you do `--help` but if you really need it you can look at the documentation and then just do `FOO=bar deletefb`.
Also I think it should be something like `RETRY_THRESHOLD` because it's shorter.
I have implemented a naive environment variable parser.
May be we can use it for MAX_POSTS too?
I also started to use static typing (mypy), I hope you don't mind. I can edit it if you don't want it.
I have implemented a naive environment variable parser.
May be we can use it for MAX_POSTS too?
I also started to use static typing (mypy), I hope you don't mind. I can edit it if you don't want it.
I'll take a look tonight (it's morning here). I'm not necessarily opposed
to using mypy but I want to make sure it fits in well with the attrs code.
On Fri., Feb. 14, 2020, 6:42 a.m. as, <notifications@github.com> wrote:
> I have implemented a naive environment variable parser.
> May be we can use it for MAX_POSTS too?
>
> I also started to use static typing (mypy), I hope you don't mind. I can
> edit it if you don't want it.
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/weskerfoot/DeleteFB/pull/115?email_source=notifications&email_token=AAC4L364SM25UFENUDK4HUDRCZ7STA5CNFSM4KUVJT22YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOELYV6IA#issuecomment-586243872>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAC4L334HXG3E33PYB2PKHTRCZ7STANCNFSM4KUVJT2Q>
> .
>
Also requirements.txt and setup.py should be updated to add the new dependency
I like the parser but did you look at something like https://pypi.org/project/environs/ as well?
Also requirements.txt and setup.py should be updated to add the new dependency
There is no new dependency as types, typing and re are included in the python3 standard library.
Actually I like the argparse interface, and I wanted to keep its consistency for the env variables. We can use environs if you like, but I think it will just weaken the readability of the code (which is the most important quality criterion to me).
There is no new dependency as types, typing and re are included in the python3 standard library.
Actually I like the argparse interface, and I wanted to keep its consistency for the env variables. We can use environs if you like, but I think it will just weaken the readability of the code (which is the most important quality criterion to me).
weskerfoot(Migrated from github.com)
requested changes 4 years ago
weskerfoot(Migrated from github.com)
left a comment
Looks fine, I just have a few small changes I want to make
Can we change it to something like this, for clarity?
# Only alphanumeric + underscore characters allowedvalid_var='^[a-zA-Z_]+[a-zA-Z0-9_]*$'ifnotre.match(valid_var,env_key):raiseValueError("Bad env key {0}".format(env_key))
Can we change it to something like this, for clarity?
```python
# Only alphanumeric + underscore characters allowed
valid_var = '^[a-zA-Z_]+[a-zA-Z0-9_]*$'
if not re.match(valid_var, env_key):
raise ValueError("Bad env key {0}".format(env_key))
```
In wall mode, even if the wall is empty, the script goes on until the for loop reaches 5000 (with 5 sec sleep each time).
In order to be more effective, I propose to try to refresh the page n times (cli and setting parameter, default n=3), and if the delete button is not found, then, we break the for loop.
I think this should be an environment variable instead of an option, with a default of 3, what are your thoughts?
I rarely use env variables, but if others do I can also check for os.environ["DELETEFB_TRY_BEFORE_QUIT"] for example. It is no big deal to me. Maybe it worth considering of a user configuration file also ? ~/.config/deletefb.yml for example ?
Yeah I just want to avoid having too many options for things that people will rarely need to use. Environment variables are nice because you won't see it when you do
--helpbut if you really need it you can look at the documentation and then just doFOO=bar deletefb.Also I think it should be something like
RETRY_THRESHOLDbecause it's shorter.Also I think the docs need to be updated...I can do that in another branch though
I see, Don't merge it, tomorrow (it's night for me), I will change it to use an env variable with the name you propose.
I have implemented a naive environment variable parser.
May be we can use it for MAX_POSTS too?
I also started to use static typing (mypy), I hope you don't mind. I can edit it if you don't want it.
I'll take a look tonight (it's morning here). I'm not necessarily opposed
to using mypy but I want to make sure it fits in well with the attrs code.
On Fri., Feb. 14, 2020, 6:42 a.m. as, notifications@github.com wrote:
Oh, it fits well ! https://www.attrs.org/en/latest/types.html
I like the parser but did you look at something like https://pypi.org/project/environs/ as well?
Also requirements.txt and setup.py should be updated to add the new dependency
it looks like environs might be compatible with mypy as well (or marshmallow if you're familiar with it)
There is no new dependency as types, typing and re are included in the python3 standard library.
Actually I like the argparse interface, and I wanted to keep its consistency for the env variables. We can use environs if you like, but I think it will just weaken the readability of the code (which is the most important quality criterion to me).
Looks fine, I just have a few small changes I want to make
action="store_true",dest="help_env",default=False,help="Print help about environment variables"Let's change it to
"Print out environment variable usage"I think there might be a better way to make it so that
--help-envdoesn't require these. I'll have to look at the argparse docs again though.from types import SimpleNamespaceclass EnvVar:Could you make it into an attrs class, similar to the other ones in https://github.com/weskerfoot/DeleteFB/blob/master/deletefb/types.py ?
default: ty.Optional[str] = None,help: ty.Optional[str] = None):assert re.match('^[a-zA-Z_]+[a-zA-Z0-9_]*$', env_key), "Bad env key {0}".format(env_key)Can we change it to something like this, for clarity?
Reviewers