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

Revert "Move lib and scan modules into a package" #12828

Merged
merged 1 commit into from
Jul 7, 2021

Conversation

WalterBright
Copy link
Member

Reverts #12766

@dlang-bot
Copy link
Contributor

Thanks for your pull request, @WalterBright!

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#12828"

@WalterBright
Copy link
Member Author

Stop dismissing my changes requested.

@WalterBright WalterBright reopened this Jul 7, 2021
@WalterBright WalterBright merged commit 63e3d57 into master Jul 7, 2021
@WalterBright
Copy link
Member Author

I was pretty clear about vetoing the PR.

@wilzbach
Copy link
Member

wilzbach commented Jul 7, 2021

Goodbye 👋

@kinke
Copy link
Contributor

kinke commented Jul 7, 2021

What a shame. So it's 1 vs. a whole bunch of core contributors, but the dictator wins? C'mon.

@dkorpel
Copy link
Contributor

dkorpel commented Jul 7, 2021

Goodbye 👋

☹️

What a shame. So it's 1 vs. a whole bunch of core contributors, but the dictator wins? C'mon.

To be fair, we are talking about the creator of the language here. Walter made more than double the commits to this repository than those 6 contributors combined, not to mention all the prior work and work in other areas. He deservers agency.

@PetarKirov
Copy link
Member

@WalterBright at the of day this is about working effectively as a team a finding a common ground versus "no, because I said so". That's quite disappointing...

@WalterBright
Copy link
Member Author

I gave several reasons. Both in the PR and in the many discussions where this has come up. Finding common ground isn't just dismissing my reviews.

@CyberShadow
Copy link
Member

Not approved. See many discussions about this, in the n.g. and at DConf.

I gave several reasons. Both in the PR and in the many discussions where this has come up. Finding common ground isn't just dismissing my reviews.

I don't know what the contents of those discussions are, but if there are technical reasons for why the reference implementation of the language cannot be scaled up, then that seems like a very serious problem that needs to be attacked head-on.

@WalterBright Are there Bugzilla issues for whatever problems are blocking improvements like these?

@PetarKirov
Copy link
Member

PetarKirov commented Jul 7, 2021

Yes, you're right, finding a common ground is not about dismissing anyone's reviews, so #12766 shouldn't have been merged before coming to an agreement. I understand if you felt bad that the PR was merged "behind your back" (not sure if that's the right way to put it).

On the other hand, it seems like you also dismissed the opinion and experience of the other parties involved without trying to understand why we saw things differently.

Since you mentioned the "Agile Software Development, Principles, Patterns, and Practices" book by Robert Martin, I think it's important to point out one principles behind the Agile Manifesto:

Build projects around motivated individuals. Give them the environment and support they need, and trust them to get the job done.

By dismissing the efforts of improving the quality of the codebase of active contributors, simply because they don't align exactly with your views, your risking demotivating and loosing them. Sure, this is not a democracy, but D couldn't have achieved what it has, without the efforts every single contributor so far.

I gave several reasons. Both in the PR and in the many discussions where this has come up.

Your reasons boil down to:

Not having a fake package hierarchy that the code implementation does not remotely respect.

@Geod24 asked you what is fake about dmd.lib, but you didn't answer. IMO, there's no thing fake about it, quite the opposite. It hides the lower-level implementation details (lib{elf,mach,mscoff,omf}) under a higher-level abstract interface - src/dmd/lib/package.d. Sure, that doesn't fix all structural problems in dmd, but it is a step in the right direction. After all, small PRs are always preferable than big ones. Later on, (if that's what you prefer based on the book that you recommended other people to read) we can go full-on with the dependency inversion principle and have the users of dmd.lib define an abstract interface and have the modules inside dmd.lib implement it.


But to zoom out on the bigger picture: do you think that #12766 is such a bad mistake that in 5 years time we could remember it and point to it as the root of all evil? Do you really think it's worth loosing active contributors about such a tiny technical disagreement?

@PetarKirov
Copy link
Member

[..] Walter made more than double the commits to this repository than those 6 contributors combined, not to mention all the prior work and work in other areas. He deservers agency.

@dkorpel that's not a good argument. At least 1 year after @9rnsr had stopped contributing (among other things, (IIRC) the reasons where even more petty disagreements about @WalterBright refusing the to accept the code formatting changes @9rnsr had to spend enormous effort maintaining and rebasing locally after the DDMD migration), @9rnsr used to have substantially more commits (and also total number of lines changed) than @WalterBright. Does that mean that @9rnsr should have disregarded @WalterBright's opinion, since he didn't have as many commits at that time?

@dkorpel
Copy link
Contributor

dkorpel commented Jul 7, 2021

that's not a good argument

I'm not trying to make an argument, I'm just trying to show appreciation for Walter's tireless work on D at a time when everyone (including me) turns against him. Andrei said last time:

He is the project leader and by far the most prolific and longstanding contributor to the project. It is professional courtesy to let him have his say in such matters, and repeatedly attempting to undermine his decision is disrespectful.

@andralex andralex deleted the revert-12766-libpackage branch July 7, 2021 13:34
@PetarKirov
Copy link
Member

PetarKirov commented Jul 7, 2021

He is the project leader and by far the most prolific and longstanding contributor to the project.

Agreed.

It is professional courtesy to let him have his say in such matters

In general, except for the merging of #12766, from what I have observed, most contributors have had to make changes to their PRs, in order to accommodate the feedback of Walter. What frustrates many contributors is that often he's quite slow (or even stubborn) to accept feedback from others.

and repeatedly attempting to undermine his decision is disrespectful.

Agreed. That said, this goes both ways.

Andrei said last time

With due respect, while I agree with the general sentiment, I believe Andrei's view in that post is biased due to his experience in working at commercial companies. The compensation gap between software engineers working at companies like FB and open-source contributors is (and will remain to be) so huge that it is silly to even compare them.
In a commercial setting, a project leader can afford to always have it their way since the developers are financially motivated to follow the rules, even if they don't agree with them. It's not a big deal if you loose a couple of developers over disagreements, as you could always hire new ones for the right amount of money.
Even if that could work for some organizations, it's obviously ridiculous to approach purely volunteer-driven projects with same mindset.

Open-source development is in a sense much more about the community rather than the end result. Such projects can only succeed if they have a thriving community of talented developers that work effectively together. Prioritizing the opinion of a single person over that of many, can push contributors away, resulting in a net loss for everyone, despite how talented and important for the project that single person may be.


I'm just trying to show appreciation for Walter's tireless work on D at a time when everyone (including me) turns against him

I believe everyone here appreciates Walter's tireless work on D. I don't want anyone turning against anybody! I really hope everyone can find a way to resolve our differences!

@andralex
Copy link
Member

andralex commented Jul 7, 2021

@PetarKirov I've been in the academia and doing unpaid volunteer work for longer than anyone cares to know. Money has nothing to do with how a professional ought to behave. An OSS organization should be able to have leadership, hierarchy, and professionalism.

@PetarKirov
Copy link
Member

I'm sorry for the wrongful characterization. To convey my point, I should have said that the example that you gave in that post is biased towards commercial projects and that I don't think should be applied to open-source projects in general, as people in those distinct scenarios are incentivized very differently.

Money has nothing to do with how a professional ought to behave.

Agreed. But that's besides my point, which is about retaining talent. Unlike employees, volunteers are not required in any way to keep working if there's a disagreement. They can fork the project, jump on another one, or simply stop working if they don't feel like it. IMO, this makes volunteers much more precious and if community-driven organizations want to successful, they must work harder to keep them.

An OSS organization should be able to have leadership, hierarchy, and professionalism.

Professionalism and leadership obviously yes, but you can't take a typical top-down hierarchy and apply it to a volunteer driven organization and expect it to work.

@Herringway
Copy link
Contributor

@PetarKirov I've been in the academia and doing unpaid volunteer work for longer than anyone cares to know. Money has nothing to do with how a professional ought to behave. An OSS organization should be able to have leadership, hierarchy, and professionalism.

It seems to me that the only one of the three that is consistently present with D is hierarchy. Leadership is not consistent about priorities or direction, often leaving things half-finished. Professionalism seems rather vague here, but I don't think reverting a PR that reached consensus with such haste that even the CIs didn't finish looking at it counts as 'professional'.

But maybe I'm just blind and everything's peachy. After all, I'm just one of the peasantry. Who am I to judge?

@WalterBright
Copy link
Member Author

haste

This has been discussed over and over for years, and I participated in those discussions. There was no haste. Consensus was not reached, and my objections to the PR were simply dismissed.

@WalterBright
Copy link
Member Author

Kenji and I had, over time, increasing differences of opinion over his pull requests. I found it very hard to understand just what changes he was making, especially as they included them very large and complex refactorings. He never shared what motivated him to leave, but it was clear that something had to give. His PRs ultimately left behind many regressions that I found particularly challenging to fix. I think a couple still remain, and another lingering problem is that nobody really understands 100% how templates get instantiated.

Kenji is a smart and amazing developer. But the way he wanted to do things just didn't work for us.

We're better off in the long term with pedestrian but solid code, rather than amazing but inscrutable code.

I do try to take my own medicine. In the last few days, it has become clear that the implementation of DIP1000 was overly complex. This has led to at least two bugs, and I suspect more that are undetected. I have undertaken a redo of parts of it to make it much easier to read the code, and eliminate the conditions that were causing the confusing problems.

My own personal goal is to write code that is so straightforward that anyone looking at it would think "Pshaw, anyone could have written that!"

@andralex
Copy link
Member

andralex commented Jul 8, 2021

Professionalism and leadership obviously yes, but you can't take a typical top-down hierarchy and apply it to a volunteer driven organization and expect it to work.

We used to believe the same, and have historically allowed a lot of unsavory things to happen in the name of "but they're a contributor". We were essentially thankful for any contribution and considered the occasional unpleasant behavior, no matter how crass, a cost to be accepted.

After 15 years of accumulated experience with that, I think we were wrong.

It is perfectly healthy for an organization of unpaid volunteers to exercise certain authority in how things are conducted, demand collegiality and professionalism, and fire people who would make it a point to cross the line. (Yes, fire even if they are not paid - we used to think that's paradoxical, but it isn't.) Looking back, being proactive about a couple of cases would have saved our community of a lot of toxic behavior.

@thewilsonator
Copy link
Contributor

Oh no you Don't.

Consensus was not reached, and my objections to the PR were simply dismissed.

Community consensus has been reached. Your objection were dismissed last time, and they have not changed since, therefore they should be rejected this time. IIRC they were: grep doesn't work well across directories (to which the response was use git grep) and some comments about the underperformance of your editor.

You mention in #12825 (review) that "DMD is not ready to be packagized" so I assume you realise packages are a case of when not if.

Being "package ready" is not a boolean property for DMD as a monolithic codebase. It is a spectrum per package, with a badness score dependant on how many things modules in that package import things they shouldn't (e.g. AST should not import semantic).

Consider this a community counter veto, if you really don't like that, then you can choose your ego or the community, because you will lose contributors if you block a reversion of this PR.

@thewilsonator
Copy link
Contributor

@andralex the same goes for you and walter.

@andralex
Copy link
Member

andralex commented Jul 8, 2021

@andralex the same goes for you and walter.

@thewilsonator I'll let others judge me; as far as I can tell, @WalterBright's treatment of others has been an example to follow.

Community consensus has been reached.

@WalterBright is a part of the community as well.

FWIW I do agree that the grep argument does not stand scrutiny and that it would be good to carefully divide dmd into packages (btw a practically-motivated attempt is here, though I'm not getting my hopes too high). But even without getting into details, we have the project leader strongly disagreeing on a topic, for reasons that others consider silly. Which of these paths is not cool?

  • Seek a workable compromise
  • Let it be and figure how to work with the annoyance
  • Decide the annoyance makes things unworkable for you and leave the project
  • Abuse the commit power the leader themselves granted you to dismiss their review and merge against their will

I should add that there was exactly one time in 15 years I could not work out a compromise with @WalterBright, although my views are often not aligned with his. That one time was with regards to @safe by default. To this day I regret the words I wrote back then, even though I "won" the argument. Wasn't worth it.

@Geod24
Copy link
Member

Geod24 commented Jul 8, 2021

@WalterBright is a part of the community as well.

This is the assumption under which I merged the PR. But if the opinion of a single person outweighs the opinion of everyone else, then that person isn't part of it, but above it.

@thewilsonator
Copy link
Contributor

@thewilsonator I'll let others judge me; as far as I can tell, @WalterBright's treatment of others has been an example to follow.

Hardly. As I recall, most of the reason contributors have left can be directly attributed to Walter's stubbornness and uncompromising and rude interactions. I don't recall a single time that I wouldn't have considered leaving under the same circumstances. I remember Walter saying he was wisely never promoted to management. That speak volumes.

btw a practically-motivated attempt is here,

excellent

though I'm not getting my hopes too high

(that has too may possible meanings written, i.e. without tone and emphasis)

Seek a workable compromise

I would love to do this, you can't reason with grep...

Let it be and figure how to work with the annoyance

... and this has been going on for how long? I have limits to my patience, and the lack of organisation is significant barrier to entry and the opportunity cost this represents accumulates.

Decide the annoyance makes things unworkable for you and leave the project

Do not tempt me, but also, do not be surprised if the community forks DMD. This is not an empty threat.

Abuse the commit power the leader themselves granted you to dismiss their review and merge against their will

Is this a community or a dictatorship?

@andralex
Copy link
Member

andralex commented Jul 8, 2021

Is this a community or a dictatorship?

That's a false choice, and put in a demagogical way to boot. The plain fact is @WalterBright has veto power and others don't. It is up to each contributor to decide whether that works for them or not.

@andralex
Copy link
Member

andralex commented Jul 8, 2021

@WalterBright is a part of the community as well.

This is the assumption under which I merged the PR. But if the opinion of a single person outweighs the opinion of everyone else, then that person isn't part of it, but above it.

Did you ask @atilaneves what he thinks?

@Geod24
Copy link
Member

Geod24 commented Jul 8, 2021

But even without getting into details, we have the project leader strongly disagreeing on a topic, for reasons that others consider silly. Which of these paths is not cool?

  • Seek a workable compromise

What workable compromise can we seek for a boolean decision ? The issue is refusing to use a non-flat directory structure altogether. If the issue was about the structure, it would be a very different discussion.

  • Let it be and figure how to work with the annoyance

Let's not pretend this is the first time this had come up. I first recall this being discussed 5 years ago. The lib change have been proposed by @thewilsonator 2 years ago. There were previous attempts, including this from 3 1/2 years ago.

Now if you look at the contributors' reactions over the years, you can see that everyone starts with nothing but respect with @WalterBright 's decision:

What has changed ? There was no reasonable motivation provided for the rejection. "Moving stuff around provides the illusion of progress", "encapsulation should be improved first" are reasonable motivations. But the grep argument seems to be the only one holding us back. There was also this discussion, which I will not repeat here but am referencing for completeness.

  • Decide the annoyance makes things unworkable for you and leave the project

If contributors leave the project as a result of this discussion, it won't be because packages are not used. It will be because of a pattern of behavior that has been there for years.

  • Abuse the commit power the leader themselves granted you to dismiss their review and merge against their will

I'll try not to take offense at this, being the abuser. Yes, the review was dismissed, but not ignored. It had been thoroughly answered by multiple party, and, in the absence of a workable compromise, nor the sight of its possibility, I decided to go ahead and merge the PR, but not before giving multiple warnings (the "72h" label was applied 5 full days before the merge, and a ping was given to @WalterBright the day before the merge).

This was done because the following conditions were met:

  • Only one person was disagreeing;
  • While the disagreement came from the BDFL, it was not a language matter;
  • The reasons given by the disagreeing party have been answered;
  • Enough time had been given for counter-arguments to the disagreeing party;
  • There was an overwhelming majority in favor of the PR;
  • Many of the reviewers were long-standing contributors;

@andralex
Copy link
Member

andralex commented Jul 8, 2021

What workable compromise can we seek for a boolean decision ?

#12832

I'll do my best to push it through.

@thewilsonator
Copy link
Contributor

Which of these paths is not cool?
Abuse the commit power the leader themselves granted you to dismiss their review and merge against their will

Is this a community or a dictatorship?

That's a false choice, and put in a demagogical way to boot. The plain fact is @WalterBright has veto power and others don't. It is up to each contributor to decide whether that works for them or not.

That is a question, that determines the answer to the one you asked. If it's anyone's choice, its Walter's. From the way you have answered it, it seems like the latter.

Whether he has veto depends on whether or not the community puts up with this behaviour. Because right now he is haemorrhaging both goodwill and contributors very quickly, both of which are in finite supply, and both take a long time to renew. If he keeps it up, he will have veto, vacuously, because there will be no community of contributors left.

I'll do my best to push it through.

I do so hope you succeed...

@ljmf00
Copy link
Member

ljmf00 commented Jul 13, 2021

As a regular contributor to the language, I see this and other decisions as very disappointing. If I didn't like D so much, I would do what others have already done.

It is clear that separating this compiler in small pieces or at least organized them would help new people to contribute and increase the overall productivity of existing contributors.

Agreed. But that's besides my point, which is about retaining talent. Unlike employees, volunteers are not required in any way to keep working if there's a disagreement. They can fork the project, jump on another one, or simply stop working if they don't feel like it. IMO, this makes volunteers much more precious and if community-driven organizations want to successful, they must work harder to keep them.

Completely agree. Taking care of the community is crucial for the success of D.

I could rant about this decision, but I'm tired of it. Just a few words: Take care of your precious community, while you have it.

@ryuukk
Copy link
Contributor

ryuukk commented Jul 14, 2021

you guys jump to conclusions and are very negative way too quickly

image

i hate seeing things like this, use words to describe what you feel, and arguments to make things change

otherwise this is just childish

walter still is the creator of the language and is still active today, of course certain changes can be hard to accept, BE COMPASIONATE

i seen the comments on the other PR, if i were walter i'd feel i have been ignored, dismissed, and to be frank; insulted too, because commenters knew exactly walter was against the changes, you guys failed to provide enough arguments and went with the bruteforce, it is the exact same as this PR's behavior, so don't be surprised ;)

anyways, keep discuss, share and oppose arguments, but don't be a kid dismissing authors feeling only because you want to weight yours more, nothing needs to be instantly merged as quick as 72h later ;) give time to find consensus by discussing more

my 2 cents

@MoonlightSentinel
Copy link
Contributor

Personal attacks (targeting contributors or not) are not appreciated.
It's easy to dismiss the reaction while ignoring the entire background leading to this PR. The arguments sorrounding proper project structure were laid out in the PR and even more in the previous incarnations. The persons involved in the previous attempts even offered to work out a compromise but that did not result in anything tangible either.

use words to describe what you feel, and arguments to make things change

Agreed but that also requires a discussion on even ground. This means that discussions should be about proper software design and without personal preferences ("Using grep -R is annoying").

Using packages to organize complex projects is well established in software engineering. Any technical shortcomings of #12766 should've been stated explicitly because they would expose failed encapsulation that could be fixed in further PR(s).

@dejlek
Copy link

dejlek commented Jul 26, 2021

Finally, why not simply call each other, and discuss this directly (or web conference whatever). If people were doing this we would probably not lose Kenji too... It is easy to be benevolent dictator, much more difficult to be a man.

@Imperatorn
Copy link
Contributor

Just found out about this.

There has to be some reason behind the decision.
It's just very hard to see the justification for reverting this.

Could someone explain?

Since when did structure become a bad thing? 🤔

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.