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

Un-deprecate setup.js #51

Closed
NotAShelf opened this issue Apr 22, 2022 · 5 comments
Closed

Un-deprecate setup.js #51

NotAShelf opened this issue Apr 22, 2022 · 5 comments
Assignees
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request help wanted Extra attention is needed

Comments

@NotAShelf
Copy link
Contributor

I have recently noticed that the setup script (npm run setup) is now heavily outdated after the config changes and ultimately, too primitive to actively implement. Additionally, the prompt library introduces a minor vulnerability that we, naturally, do not want to have in BrayanBot.

Problem is, the prompt library may be deprecated (see #223 on their repo) and the alternatives do not include the regular expression checker that I need the setup.js to be opinionated to the necessary degree.

The alternatives are:

We can either contribute to one of the alternative libraries (would good publicity) or deprecate setup.js entirely.

@NotAShelf NotAShelf added enhancement New feature or request help wanted Extra attention is needed dependencies Pull requests that update a dependency file labels Apr 22, 2022
@NotAShelf NotAShelf self-assigned this Apr 22, 2022
@NotAShelf NotAShelf changed the title Un-deprecate **setup.js** Un-deprecate setup.js Apr 22, 2022
@Resorted
Copy link

I think having a setup script is fine, if it's simple, updated and doesn't "force" the user to create anything that they don't want.
Have a simple setup script that helps the user set up the configs and enable/disable functions they would like to use.

@NotAShelf
Copy link
Contributor Author

I think having a setup script is fine, if it's simple, updated and doesn't "force" the user to create anything that they don't want. Have a simple setup script that helps the user set up the configs and enable/disable functions they would like to use.

You could say that it's simple, but it does ultimately force you to create your config.yml (which you need to create either way, so that doesn't really count as "forcing"). I'll see if I can simplify the script even further while also fixing the library vulnerability.

@Resorted
Copy link

You could say that it's simple, but it does ultimately force you to create your config.yml (which you need to create either way, so that doesn't really count as "forcing"). I'll see if I can simplify the script even further while also fixing the library vulnerability.

Yeah, when I said force user to create anything, I meant channels, roles etc.. like CB does. The creation of the config through a script is nothing but positive imo. I gave the script a read and it looked fine to me.

@NotAShelf
Copy link
Contributor Author

The best solution to re-introduce the set-up script seems to be to update the prompt library ourselves. After updating dependencies and running the tests, it seems to work as intended therefore can be used once more.

@NotAShelf
Copy link
Contributor Author

This has been practically auto-closed with the migration to V2. I will look into bringing back an install script in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants