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

improve user journey for scripting #2121

Open
2 tasks done
a-wallen opened this issue May 13, 2024 · 5 comments
Open
2 tasks done

improve user journey for scripting #2121

a-wallen opened this issue May 13, 2024 · 5 comments
Labels
enhancement New feature or request

Comments

@a-wallen
Copy link

a-wallen commented May 13, 2024

Checklist

  • I'm reporting a feature request
  • I've checked for similar feature requests including closed ones

Description

Sherlock is great from the CLI, but not as much trying to write a script using it, here's why.

  1. The documentation does not have clear instructions for calling the sherlock function with it's required arguments. Here's an example. The instructions do not clearly state how to provide a default argument, stating how to find the class that needs to be implemented.
  2. Just like the CLI tool, the sherlock function should be able to work with just the username argument. For example: the site_data could be defaulted to data.json.
  3. I don't want to suggest that the query_notifier parameter gets defaulted. It's fine the way that it is. However, there should be a separate implementation called QueryNotifierMap where you can access result as a map. QueryNotifyPrint is for the CLI, developers want to call this with just an object.
@a-wallen a-wallen added the enhancement New feature or request label May 13, 2024
@matheusfelipeog
Copy link
Collaborator

Hi @a-wallen, thanks for opening this issue ;)

Sherlock is essentially a tool focused on being used via CLI, which is why there isn't a more comprehensive documentation of its internal components.

I personally like the idea of providing clear and documented ways to integrate Sherlock into other projects; that's actually a good idea. We can improve on that over time, although it's not one of our priorities right now.

@a-wallen
Copy link
Author

@matheusfelipeog I was planning on submitting the following PRs

  • An implementation for QueryNotifierMap so that developers can just get the result as an object.
  • Improved docstring comments for the sherlock function.
  • An example in the README.md.

Hit reply to this an just "X" the ones that you want me to do.

@ppfeister
Copy link
Member

ppfeister commented May 13, 2024

While waiting for @matheusfelipeog or @sdushantha... wanted to add:

I don't believe that this documentation should be in the readme. That doc can become easily cluttered with material that doesn't apply to 90% of users. I would suggest /docs/plumbing/${stuff}.md, which can be linked to via the readme. For a basic example of what I mean, I've done something similar with the Installation section of the new readme, linking to /docs/install.md. Just keeps things tidy.

Documentation within the code is highly valued as well, as you mention, since external documentation can easily become out of date. Aside from docstrings, type suggestion for arguments and returns would be nice to have more of.

Would like to see if the others agree.

@ppfeister
Copy link
Member

Sherlock is packaged in several places and isn't likely to be ran as a single script --- it's more likely that people will install the package itself via pip or similar

When using the package, people aren't calling the sherlock function. They're calling the sherlock package. In order to call the sherlock function it'd have to be something like

from sherlock import sherlock # module within package
sherlock.sherlock(usernames) # function within module

quite verbose when

import sherlock # whole package
sherlock(usernames) # using package entry point

is probably preferred in 99% of cases.

I believe main() is the current package entry point. main() should currently take a ton of parsed sys args rather than function args. Would you be able to somehow adapt it so the entry point accepts both command line args and normal scripted function args? How difficult do you think that adaptation would be?

@a-wallen
Copy link
Author

@ppfeister yup you're on point: this is what I think you mean #2143

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants