Skip to content

Kill the Semigroup instances with fire #4073

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

Open
ezyang opened this issue Nov 1, 2016 · 3 comments
Open

Kill the Semigroup instances with fire #4073

ezyang opened this issue Nov 1, 2016 · 3 comments

Comments

@ezyang
Copy link
Contributor

ezyang commented Nov 1, 2016

They don't make any sense at all, and when on-boarding @Ericson2314 and @abooij , both wondered what the right way to implement the semigroup instance was (there isn't a right way.) Can we get rid of it? I guess there is some code that uses it to merge sections together (conditionals maybe)? Can we find another way of managing this behavior?

@23Skidoo
Copy link
Member

23Skidoo commented Nov 1, 2016

I guess you mean Semigroup/Monoid instances for Library/Executable and related types, and not, for example, flags. I actually tried doing that in an attempt to fix #3313, but at that point there was some (hairy) code that still used them.

@abooij
Copy link
Contributor

abooij commented Nov 4, 2016

For me, yeah, I found the Semigroup instances for build components and their fields confusing and inappropriate. What should the semigroup action on two Versions be? (luckily this instance does not exist, but the abundance of other instances makes it seem like this one is missing)

@23Skidoo
Copy link
Member

23Skidoo commented Nov 4, 2016

I'm +1 on removing Semigroup instances for Library/Executable et al., in case it's not clear.

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

No branches or pull requests

3 participants