-
-
Notifications
You must be signed in to change notification settings - Fork 369
[#439] Implement BDD style alias methods #662
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
Conversation
Hi, thank you for your contribution. Such high-level definitions should be very carefully considered. What should be checked IMHO:
@jamesxwhitehead can you provide more examples of its usage. |
@igorwojda maybe you also have more examples |
Hi @oleksiyp I definitely agree with your points. I implemented this feature as it is tagged as "good-first-issue" and seemed like one I could tackle. This is my very first contribution to open-source 😃 If you do not feel that this is a good feature for MockK that's definitely fine, and perhaps you could point me towards a different issue that you would like to see implemented. |
No, I am not saying it shouldn't be considered. I just say let's slow down a bit and think, bring rational arguments... again thanks! Especially for your first contribution!!! |
TBH introduction of such aliases seems rather dangerous to me. It is not a problem in small teams but ours it is rather a bigger one and we would have to put non-trivial work to:
Since these are pure aliases I'd guess that they can nicely live as an extension library or directly in your code-base. |
First of all, thanks a lot @jamesxwhitehead for sending this through. However, I agree with @hrach's point that having two different ways of doing things may introduce ambiguity and, as @oleksiyp mentions, this introduction should be carefully considered. Perhaps we could implement these as an additional module, like Then again, the last point in @oleksiyp's list is what concerns me most: do we really need these aliases that basically just rename MockK's functions without introducing any additional feature? I personally don't think we do, but I'd like to gather some community feedback on this. Also, @jamesxwhitehead, sorry for making the issue look easier than it actually was :( |
I think that adding these styles to the project could create some issues. The biggest one IMO is the potential usages of both styles in a single project (leading to code inconsistency). I think we would have a wider look ad check what the industry is doing:
Heaving various styles could be beneficial because of personal or business preferences (assuming the business is using BDD everywhere else in the company), however, we should be explicit about using them to avoid style mixing. I think we should follow @Raibaz suggestion to create separate dependencies each with its own style eg.
Nothing comes to my mind that could potentially conflict
We have
This must be closely considered
I guess the problem with aliases as oppose to separate in is that you can mix the styles. It also affects error messages, right?
My original BDD issue has many likes #439 and the existence of |
BTW Great contribution @jamesxwhitehead 🎉 |
Yes, separate module seems to be the most reasonable way to go |
I agree 100% with everything above. I will try to re-implement this feature as a new module instead and we can go from there 😄 Thanks for the great feedback. |
No description provided.