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

feat(redis-commander): add redis commander and cleanup based on guidelines #648

Merged
merged 4 commits into from
Oct 25, 2023

Conversation

Mr0nline
Copy link
Contributor

@Mr0nline Mr0nline commented Sep 7, 2023

No description provided.

@Mr0nline
Copy link
Contributor Author

@coolaj86 bumping this PR 😄

@coolaj86
Copy link
Member

coolaj86 commented Oct 16, 2023

Hey @Mr0nline I'm taking the day to get caught up on all these Webi PRs.

My concern with this one is that since it's already available via npm, and that requires node, I'm not sure what the advantage is here, other than the cheat sheet (which may be a big win).

When I did prettier, it's because it was necessary for vim-essentials. Also, prettier is used by people who are not necessarily node users (anyone who writes Markdown, for example). But if it weren't for the compatibility guarantee, I'd consider removing it.

Can you make a good case for why this should be on Webi rather than just webi node; npm install --location=global redis-commander?

(that's not a "no", but I don't want to open the floodgates to a bunch of packages that require another package manager, node, and that already install correctly cross-platform assuming that that thing is installed correctly)

@Mr0nline
Copy link
Contributor Author

Yes, you're right @coolaj86,

Redis commander is in my day to day toolchain and thought that it would streamline the ecosystem for me in which I'd do everything using webi, That's why I provided a PR 😄

I've thought that since webi always forks from the upstream sources, It would not bloat up the core webi scripts and that's why I've thought to contribute by adding my most needed yet missing tools into webi 😅 since I'm using webi for node, bun etc.

It's fine if you want webi specific for general purposes, In that case I can close this PR as well since I just wanted to do everything by webi as much as I could 😅

@coolaj86
Copy link
Member

I think the litmus test question is this:

Is it only node users who use Redis Commander, or are there people who use (or would use) Redis Commander without caring about (or knowing) that they're using Node?

That's how prettier got in: lot's of people will want to use it, even if they never know or care that they're using node.

If the audience for Redis Commander is (or could be) similar, then I think it's a good fit.

@coolaj86
Copy link
Member

Preview

See https://beta.webinstall.dev/redis-commander

@Mr0nline
Copy link
Contributor Author

Is it only node users who use Redis Commander, or are there people who use (or would use) Redis Commander without caring about (or knowing) that they're using Node?

@coolaj86 , Yes, I'd say that any one who works with Redis might want to use Redis Commander which is a GUI for Redis. Since there's a high chance that they are using some other languages for backend like python but still uses redis for queue management or caching and in that case they might want to use Redis Commander. And yes this do requires node as a peer dependency but I think people won't have any issues if they want to use commander as they already are very well aware about it being a NPM package 😄

Thanks for having it on beta 🤞

@Mr0nline
Copy link
Contributor Author

Preview

See https://beta.webinstall.dev/redis-commander

Working great 🙌

@coolaj86
Copy link
Member

I will slate this for addition. I'd like to do another batch of these later in the week.

@coolaj86
Copy link
Member

@Mr0nline Can you either sign your commits as per https://github.com/webinstall/webi-installers/blob/main/CONTRIBUTING.md#signed-commits or allow maintainer edits so that I can rebase this and sign off on it?

@Mr0nline
Copy link
Contributor Author

Mr0nline commented Oct 25, 2023

@Mr0nline Can you either sign your commits as per https://github.com/webinstall/webi-installers/blob/main/CONTRIBUTING.md#signed-commits or allow maintainer edits so that I can rebase this and sign off on it?

I think I already ticked on Allow edits and access to secrets by maintainers so I guess you could be able to make changes :)
@coolaj86 the thing is that I already have GPG on for all of my commits so I'm not quite sure how it shows unverified so better if you're able to sort it out!

Copy link
Member

@coolaj86 coolaj86 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How npm works has changed and I've since learned some better ways to do the install. I've added those, including for PowerShell in a follow-up #705.

Approved. Will rebase once the commits are signed or I can do maintainer edits.

@coolaj86 coolaj86 merged commit 874f081 into webinstall:main Oct 25, 2023
3 checks passed
@coolaj86
Copy link
Member

Thanks for your work on this.

If you create and drop a Đash Wallet address here (or DM me) and I'll be more than happy to send a tip / buy you lunch.

:)

@Mr0nline
Copy link
Contributor Author

Your welcome @coolaj86 😅

I don't have Dash wallet but I was willing to highlight a bug on Telebit (It's in fact from where I found about you and webi in first place)!

But since it's not on github, I'm not sure how I can create a issue on it, Is it fine to send a email that mentioned on telebit website or you prefer another way of communication for that issue!

Let me know and I'll share a more details on that!

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

Successfully merging this pull request may close these issues.

None yet

2 participants