BlakeWilliams / Elixir-Slack

Slack real time messaging and web API client in Elixir
MIT License
675 stars 182 forks source link

Add extra features #202

Closed yordis closed 5 years ago

yordis commented 5 years ago

I added been keeping this locally since I need these features for my Slack's Commands.

I think other people will find value from this (hopefully)

yordis commented 5 years ago

I am not sure what is going on with the CI ...

yordis commented 5 years ago

Hey @BlakeWilliams , is there any chance that we could work together on this PR?

BlakeWilliams commented 5 years ago

Hey @yordis, sorry for the delay, and thanks for contributing!

I try to keep this package pretty narrowly focused on being just API calls and allowing those using the package to implement their own patterns. Given that, maybe a better place to extract these helpers would be into some kind of elixir-slack-contrib package?

yordis commented 5 years ago

@BlakeWilliams for the greater of the community, in my opinion, I would not support splitting into many packages.

Look what happens in ecosystems like NPM, which to be fair, bytes matter because of the Web, so there is an incentive to create super tiny packages.

But regardless, the chaos to maintain packages or align the people around a specific topic (in this case, Slack) is there, and the community is what suffers the most (especially newbies). I believe that collaboration + diversity is the most reliable combination.

I would no like to be working in the same mess (unless we do not agree on this opinion).

On the other hand, the beauty of Golang where you have everything you need available in the language, and people aligned in common use cases.

I don't believe it is a good idea, especially that your namespace is Slack rather than more specific and aligned with your intention.

I have no problem extracting this and creating a package, but I discourage myself from choosing that as my first option.

I prefer to align the people and encourage collaboration about Elixir Slack on this repository, then creating multiple packages just because we want to force the separation of concerns by repositories.

But,

That is just my opinion, and this conversation could get philosophical and could lead to nowhere.

Decide this; I trust your judgments. I will beg, however, to be thoughtful about it.

In case you decide to reject the contribution.

I will suggest creating a GitHub organization for Elixir Slack packages. At least people aligned around a particular group or organization. Create multiple repositories with your suggested idea (but at that point, it is just more management without valid reasons in my opinion).

Cheers,

BlakeWilliams commented 5 years ago

@yordis I appreciate the concerns and see your point, but I don't believe this is similar to NPM.

This library is the Slack library because it provides a a minimal, but viable API for people to use to create their slack applications. There's a decent number of ways that you could make this package higher level but then we're choosing the trade-offs for the users of the package and also causing us to have a higher maintenance bourdon.

I have no interest in splitting the existing package into multiple packages, but I also don't think the library should provide opinionated tools on how to solve application code problems.

I really appreciate you opening the PR, but this is something I think would be more useful for a package a level or two higher than this package to implement, e.g. a package that makes writing Slack bots incredibly easy by abstracting away some of this libraries boilerplate.

yordis commented 5 years ago

@BlakeWilliams What about taking the basic data structures like Command, Message and remove the Handler specific code?

Since that could be done with multiple strategies based on requirements but the data structures are the same for everyone.

yordis commented 5 years ago

@BlakeWilliams sorry, would you mind to share your thoughts about the last comment?

BlakeWilliams commented 5 years ago

@yordis Sorry, must have missed that message.

I still feel like that kind of code would be better suited for a third party package, especially if they're just for supporting application code but are entirely optional and not used or suggested by the core package.

yordis commented 5 years ago

But those are a data structure that will never change, like Slash Command, Message, Message Block and all those things.

And almost 95% of the code from this package will be optional since we only use a few things from it.

BlakeWilliams commented 5 years ago

And almost 95% of the code from this package will be optional since we only use a few things from it.

That is true, but the focus of this library is to provide access to Slack API's via Elixir. I like keeping that focus pretty narrow and I feel that adding those structs would start to lose that focus a bit. Like I said, I think it's great for application code or a third party library, but it's not something I want to add to this library given the previously mentioned focus and added maintenance.

If it helps at all, when I see those structs I think that a package that builds on-top of this library to provide a better bot interface or something similar would be rad. Unfortunately I don't have much interest in maintaining something like that or expanding this library's scope to include that extra scope.