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

Can you please avoid copying files from other repositories? #313

Closed
benjamn opened this issue Dec 17, 2014 · 20 comments
Closed

Can you please avoid copying files from other repositories? #313

benjamn opened this issue Dec 17, 2014 · 20 comments
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@benjamn
Copy link
Contributor

benjamn commented Dec 17, 2014

For example: https://github.com/6to5/6to5/tree/master/lib/6to5/transformation/transformers/es6-generators

You're effectively forking the code, and making it much more difficult for you to incorporate updates quickly (or contribute changes upstream). It's unclear what version of Regenerator these files even represent. I don't understand the benefit you imagine this has for 6to5 or any of its contributing libraries. This isn't how open source should work!

Sorry for the alarmist tone, but this is a real problem. I'm happy to work with you to make Regenerator easier to use as an NPM package.

Thanks.

@sebmck
Copy link
Contributor

sebmck commented Dec 17, 2014

Previously regenerator was in a fork regenerator-6to5 which was in a now deleted GitHub repo. The initial reason for moving the repo was that I didn't want to have to maintain multiple projects simultaneously, I know that's a bullshit reason and it was a off-the-cuff decision.

6to5 has been massive for me. I've never maintained or handled an open source project let along one so complicated and dense. I'm basically the only core contributor and the ever so awesome @thejameskyle helps out with issues, website, docs, org etc. I still find it really intimdating to be running a project that people actually use. I love that people are using my code and project and I try my best to have awesome support, so supporting someone elses code, especially for such a big feature feels weird to me.

Part of the reason for inlining regenerator was so I could iterate on it and make it fit into 6to5 far more easily, stuff like supporting the small acorn variations and keeping uids more consistent. Maintaining 6to5 is a big task and I'm lazy so even just having to maintain our fork of acorn is hard for me let along maintaing multiple forks. Ever since I added generator support via regenerator I've always felt kinda iffy about it because it's the only part of the 6to5 transformation process that I delegate to another module. The only reason I used regenerator was because users were requesting generator support and regenerator did it extremely well. I don't want you to think I'm not appreciative of your work, I really am and really look up to and admire you guys. I just feel like a little fish in a big pond and it's made it hard for me to contribute outside of my own bubble.

I guess I'm pretty prudent when it comes to these decisions since I'm only just getting used to this sort of stuff. Just a few months ago there was no 6to5, I had no experience with parsers or even anything close to that. 6to5 has been a big step for me and I've made a few spur of the minute decisions that in hindsight I wish I handled differently. It's been a massive learning experience and the initial goal was to write my own generator implementation, not because I think I can do it better but just because I love writing code.

I'm not sure what you're most concerned with, is it that you think I'm misrepresenting regenerator by inlining it and making a bunch of modifications or that you just don't want duplication of effort? I understand both and I'm unsure of if you want the changes merged upstream or if you'd just be fine with a fork. I mean no ill-will and I don't want you to misinterpret my intentions.

@chrisabrams
Copy link

I'm having trouble figuring out what the cause of the problem is...is it that you are having trouble keeping multiple libraries forked versions' sync'd with this project because they are in different directories? If that is the case, I'd be happy to help you setup a way to keep them in sync. Otherwise, I don't follow what the problem is :(

Really happy for 6to5 though; keep up the great work :)

@sebmck
Copy link
Contributor

sebmck commented Dec 17, 2014

@chrisabrams It's not maintaining multiple modules/directories but just keeping them released and keeping a track of versions etc. When I did have the regenerator fork included it wasn't published to npm and was inlined in package.json with a link to the tar.gz.

@teliosdev
Copy link

You're effectively forking the code, and making it much more difficult for you to incorporate updates quickly (or contribute changes upstream).

Actually, forking the code allows him to make changes to your code without having to have it go all the way back upstream (and, in the cases where you're going to reject his changes, this is good). "Forking the code" is one of the best things about open source, and shaming it is exactly what you don't want to do.

It's unclear what version of Regenerator these files even represent.

When you fork and modify a code base to a large enough extent, it makes the version of the code base meaningless.

I don't understand the benefit you imagine this has for 6to5 or any of its contributing libraries.

Maybe that's where the problem lies.

This isn't how open source should work!

Don't get me started on this. Open source is about allowing people to make modifications to code freely and openly, and to create derivative works for the good of the whole. In this case, he copied the entirety of your code base into his repo because he was having difficulty working with and getting your code to work well with his code. Open source doesn't require anyone to do anything, and that's the beauty of it. You don't have to do anything in open source, and that's brilliant. I don't have to make a pull request. I don't have to follow conventions. I don't have to use libraries.

It's the same here. You don't have to use 6to5. If you don't like it, then you can fork off.

@chrisabrams
Copy link

Well said @medcat 👍

@benjamn
Copy link
Contributor Author

benjamn commented Dec 18, 2014

@medcat Everything you say is true, and I don't even mind that you completely mistook my intentions and think I need to "fork off." We all misread text on the internet from time to time, and we respond to what we think we read, before asking for clarification.

Forking is great! Open source is all about forking, but here's the key point: forking works best when changes can be shared between the fork and the upstream repository in a sustainable, frictionless way. Is that controversial? I hope not.

That said, what's happening in this repository is not exactly a fork. In my mind, a fork is a copy of some version of another repository with some additional changes on top. Where are the tests? Are you running Regenerator's very comprehensive test suite when you make changes to the code? If not, where does your confidence in those changes come from? How am I supposed to test my upstream changes against 6to5, to make sure I haven't broken some behavior that you're relying on? That's something that's really important to me! How are you supposed to rebase against upstream? How are you supposed to submit PRs, so that I know when you're having problems?

If none of that is important to you, then I don't know how we can have a meaningful conversation, because we just don't share the same values at all. But I think those things probably are important to you, or you wouldn't be an active user of a website that's all about social coding.

I don't need @sebmck to stop keeping a copy of these files. This is all MIT-licensed code, so he can legally do whatever he wants with it. My concern is simply that this whole adventure in ES6-to-ES5 transpilation will work so much better for everyone involved if we have clear places where discussions happen, experiments are attempted, and solutions accumulate.

Read @sebmck's previous comment, please. It sounds like he was struggling with semantic versioning and NPM package wrangling, and I share his pain! I've done everything I can to respect semantic versions, and I'm more than happy to expedite releases to accommodate the needs of developers and library authors who want to use Regenerator (or any of my other libraries, for that matter). I care deeply about that.

To say that it's difficult to get changes merged into Regenerator, and so the only way forward is to fork and forget, is so far from my understanding of this situation that I have to assume you're simply conjuring a strawman. I don't know what more to say, except that you're accusing me of something I really don't feel guilty of. Feel free to back that accusation up with any evidence at all. Or we can just drop that particular line of the conversation. That's fine too.

If I submitted a pull request to 6to5 that restored the independence of Regenerator as a separate package, would that be welcome?

@sebmck
Copy link
Contributor

sebmck commented Dec 18, 2014

@benjamn

Where are the tests? Are you running Regenerator's very comprehensive test suite when you make changes to the code?

Yes.

To say that it's difficult to get changes merged into Regenerator, and so the only way forward is to fork and forget, is so far from my understanding of this situation that I have to assume you're simply conjuring a strawman.

That's not even close to what I said and you completely misinterpreted what I actually meant.

If I submitted a pull request to 6to5 that restored the independence of Regenerator as a separate package, would that be worthwhile?

A fork is always going to be necessary.

  • esprima-fb and recast bloat up the dependency tree significantly which is a concern because 6to5 doesn't even use them and they're included in the compiled browser version.
  • Slight AST variations and non-standard AST types that 6to5 uses. (This is the main one)
  • Different UID formats which goes completely against the 6to5 philosophy of outputting readable and consistent code.
  • There's a significant performance hit with ast-types which is used extensively in regenerator. 6to5 performance is already a big enough concern without having to worry about the performance of an external dependency.

Maintaining changes really isn't that difficult and I'm definently up for it so I'm not sure what the concern is there.

At the end of the day I'm the one who has to support this. It's already hard enough and I'm definently up for it but I've made decisions that have lead to easier iteration and maintenance.

I'm a bit more concerned with how you went about raising these concerns with me. Would've been nicer to get an email or privately contacted me via a variety of other means.

@teliosdev
Copy link

@medcat Everything you say is true, and I don't even mind that you completely mistook my intentions and think I need to "fork off." We all misread text on the internet from time to time, and we respond to what we think we read, before asking for clarification.

Yes, because the entire issue stems from me completely mistaking your intentions and misreading your text, even though I read it thoroughly roughly three to five times before I decided to make my comment, and then another three to five times to make sure I responded to each part individually. I also tried to ascertain the exact context of what you were trying to demonstrate in your conversation, because I wasn't sure how you were meaning to approach it - perhaps you were legitimately concerned with @sebmck's workload in terms of working with regenerator? So I checked out other places to gather information about the situation - for example, the pull request so conveniently linked in the history, which I'll link here: facebook/regenerator#156. This line is particularly delicious:

I'll consider fixing this some other way if @sebmck agrees to stop storing modified Regenerator source files in his repository: #313

Huh. Well then.

Forking is great! Open source is all about forking, but here's the key point: forking works best when changes can be shared between the fork and the upstream repository in a sustainable, frictionless way. Is that controversial? I hope not.

Yes. Let me start with the "changes" bit - there's many reasons to fork something. One of them is to make changes to the repository. Another one might be to mirror it for purposes. Other purposes might include changing the ownership of the repo, because someone disagreed with the owner's methodology, or making local changes that the author of the changes didn't want to be merged back to the upstream. Forking doesn't have to be anything about sharing changes. It also doesn't have to be about the upstream repository either.

That said, what's happening in this repository is not exactly a fork. In my mind, a fork is a copy of some version of another repository with some additional changes on top.

Which is exactly what this repository is, see:

Where are the tests? Are you running Regenerator's very comprehensive test suite when you make changes to the code? If not, where does your confidence in those changes come from? How am I supposed to test my upstream changes against 6to5, to make sure I haven't broken some behavior that you're relying on? That's something that's really important to me!

I'm sorry, you did see this entire directory? I'm sure someone of your statue didn't happen to have missed it, right? You were saying something about tests being really important to you, so I just thought I'd make sure you didn't miss it.

(Also, was that an attempt at selling Regenerator by putting "very comprehensive" in front of "test suite"? Nice.)

How are you supposed to rebase against upstream? How are you supposed to submit PRs, so that I know when you're having problems?

There always is the possiblity of git submodules, but I haven't heard good things from those. And as for PRs, he could just copy his changes over to another repository based on your repository, and submit them anyway; or, if he's having problems that he doesn't have a fix for, he could just submit an issue, because those exist.

If none of that is important to you, then I don't know how we can have a meaningful conversation, because we just don't share the same values at all. But I think those things probably are important to you, or you wouldn't be an active user of a website that's all about social coding.

Because not having the same values at all means not being able to have a meaningful conversation. I think it says loads about the quality of the person when they're able to hold a meaningful conversation while not having similar values. It builds character, I'd think.

I don't need @sebmck to stop keeping a copy of these files. This is all MIT-licensed code, so he can legally do whatever he wants with it.

You missed the whole point of the "you don't need to do anything" if you think I was talking about the legal aspect of it.

My concern is simply that this whole adventure in ES6-to-ES5 transpilation will work so much better for everyone involved if we have clear places where discussions happen, experiments are attempted, and solutions accumulate.

My concern is simply that your argument seems to have a gap in logic - I'm not sure how @sebmck not storing a copy of these files prevents a clear place where discussions to happen, prevents experiments from being attempted, and prevents solutions from accumulating.

Read @sebmck's previous comment, please. It sounds like he was struggling with semantic versioning and NPM package wrangling, and I share his pain! I've done everything I can to respect semantic versions, and I'm more than happy to expedite releases to accommodate the needs of developers and library authors who want to use Regenerator (or any of my other libraries, for that matter). I care deeply about that.

After all of the trouble you went through to explain that reading text online may lead to misunderstandings, you want to talk to me about the contents of someone else's text online? Alright. The problem doesn't come from your versioning; the problem comes with versioning itself. The issue, from my understanding, arose from having to keep track of separate versions of modules at the same time. It's not about requiring releases to be expedited, it's about releases in the first place.

To say that it's difficult to get changes merged into Regenerator, and so the only way forward is to fork and forget, is so far from my understanding of this situation that I have to assume you're simply conjuring a strawman.

No, that's you attempting to conjure a fallacy and in turn conjuring a strawman of your own. The problem is versioning itself, so getting changes merged into Regenerator would not help at all in this case.

I don't know what more to say, except that you're accusing me of something I really don't feel guilty of. Feel free to back that accusation up with any evidence at all. Or we can just drop that particular line of the conversation. That's fine too.

What, exactly, is the point of this? You're not getting anywhere with this, you're not helping your argument at all, and in fact, you're just making yourself look like an ass. Please try to refrain from saying such things in the future.

If I submitted a pull request to 6to5 that restored the independence of Regenerator as a separate package, would that be welcome?

I don't think you understand the problem if you're going to attempt to give a solution like that.


Seriously, your response to the whole situation has been absolutely terrible. Your words "completely mistook my intentions" I think apply completely and whole heartedly in this context - you should have contacted the owner of the repository first and foremost to understand his reasonings for doing such a thing, and if you then both agreed that this needed to be publicly debated on, created an issue. You could have also presented it in a manner that would have been, for the lack of a better term, nicer, and probably would have been considered more if it had not been for your ugly tone.

And, if I may remind you, @sebmck's use of Regenerator's is not only not against your LICENSE, but it also harms you in no way, and doesn't make your job any more difficult; it was his choice to include it in this manner, and if he had felt that this was an issue that could have been resolved on your part, he could have contacted you. Did you even read the license that you included in your code?

@benjamn
Copy link
Contributor Author

benjamn commented Dec 18, 2014

I take all of your points to heart, @sebmck and @medcat, and I sincerely regret my initial tone.

If you're interested in making further progress here, beyond putting me in my place (which you've done, and I actually appreciate it), please forgive me for not spending any more time defending myself. This is not an attempt to deflect criticism. I started this, and I'm sorry. If you would like to tell me something more about my my personal bearing or argumentative style, please find another channel. I really regret that I didn't start this discussion with a private email to @sebmck. That would have been vastly more productive!

You're both right that I totally missed that directory full of tests. My oversight was in part because the directory layout is different from the corresponding files in the Regenerator repository, and I didn't automatically know where to look, but I certainly should have looked harder! Your test framework is different and actually quite a bit nicer than mine, so while it may not be entirely trivial to share changes between the two, I don't blame you for doing it your way.

It still saddens me that all this work was ultimately easier than just using Regenerator as an NPM package, and I feel like that's my fault. Thanks for the constructive feedback about why the fork is necessary, @sebmck. I wish you could benefit from future performance improvements on my end, but I don't blame you for exploring different techniques for AST transformation. It's a hard problem, and I certainly have not solved it.

Is there anything I can do to make your maintenance burden easier, @sebmck? The offer of a PR was not a boast or a threat. I thought you might have appreciated a different approach to keeping this code modular. The offer still stands, if it is welcome. If not, that's less work for me, so I really don't mind either way.

@medcat For what it's worth, I've edited that comment: facebook/regenerator#156 (comment)

I'm closing this issue because it doesn't sound like we're heading towards un-forking, and that's honestly fine. It's obvious that you didn't take the decision to fork lightly, given how much work it must have been, and you know your maintenance burden better than I do.

Best of luck to you.

@sebmck
Copy link
Contributor

sebmck commented Dec 18, 2014

@benjamn There are definently ways that collaboration can be improved and I apologise for not reaching out to you. There are a few issues that I've found that I'll report on the regenerator repo. There is possibly a way that 6to5 can switch completely to regenerator (without a fork or inlining it) with a few additional options etc, I'll get back to you on that since if it's possible I'd be very keen.

@stefanpenner
Copy link
Member

Open source enables us the freedom to experiment, explore and sometimes come up with novel solutions. Our work can easily stand on the shoulders of our peers work. This is thanks to their generosity and openness. But remember, without re-investment (when it makes sense) we break the cycle and the potential.

This cycle can be broken on both ends. Maintainer, contributor must work together to share the burden, so everyone benefits.

6to5 has been massive for me. I've never maintained or handled an open source project let along one so complicated and dense.

6to5 is a great accomplishment, especially for a first time maintainer of such a project, good job! Your making the web better.

I'm basically the only core contributor

But you don't have to go it alone, I know for a fact the entire ecosystem is starving for more collaboration as everyone is bound by time, and the only thing this will do is improve the end users experiences.

Relevant discussion: regarden escodegen + esotope estools/escodegen#211 (comment)

I realize the upfront effort in collaboration is higher, but the multiplier of a healthy community far outweighs the ability of a single contributor.

Whatever the future holds, I hope it holds more collaboration and re-investment.

@sebmck
Copy link
Contributor

sebmck commented Dec 18, 2014

@stefanpenner

But you don't have to go it alone, I know for a fact the entire ecosystem is starving for more collaboration as everyone is bound by time, and the only thing this will do is improve the end users experiences.

Yeah definently. I just want to clarify that I'm completely up for the task and I understand that the barrier to entry to something like 6to5 is incredibly high. I don't want to belittle the contributions by other contributors (such as @douglasduteil who's working on awesome spec compliant SystemJS module formatter #307), I really appreciate any contributions even if they're just adding a line to documentation.

@sebmck
Copy link
Contributor

sebmck commented Dec 28, 2014

As of 2.0.0 vanilla regenerator is used.

@benjamn
Copy link
Contributor Author

benjamn commented Dec 28, 2014

Awesome!! I really hope this means less work for you, moving forward. Feel free to ping me any time I can help.

@stefanpenner
Copy link
Member

👍

@sebmck
Copy link
Contributor

sebmck commented Dec 28, 2014

I have to do some weird stuff with patching ast-types though see patch.js.

@benjamn
Copy link
Contributor Author

benjamn commented Dec 28, 2014

For what it's worth, that's exactly the way I hoped folks might extend ast-types, so 👍 to that. There's been some talk about allowing extensions that aren't visible to other clients of the library, but that's still in the brainstorming stage at this point.

@sebmck
Copy link
Contributor

sebmck commented Dec 29, 2014

It's just annoying because I have to maintain ast-types version consistency with recast.

@benjamn
Copy link
Contributor Author

benjamn commented Dec 29, 2014

Idea: you might be able to stop depending on ast-types directly and just grab/extend require("recast").types, so that you're guaranteed to be using the same version of ast-types that recast is using.

@sebmck
Copy link
Contributor

sebmck commented Dec 30, 2014

There's no way to get the version of recast that regenerator is using though unless something dodgy and unreliable like require("regenerator/node_modules/recast"); is used.

@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 19, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

6 participants