The Rails foundation has been talking about improving docs for a while now [@AmandaPerino ], and that’s partly what encouraged me to make the effort to submit a couple of changes.
It started well
I hit an issue with some unexpected behaviour. As far as I could see, this wasn’t documented - though I could see where it came from in the code.
A helpful core team member pointed out that this was a duplicated issue, and the dupe explains why this is expected behaviour.
The same person commented that a documentation PR would be welcome.
I asked for input on how to format that (which was rapidly forthcoming).
I submitted a PR and it was merged a couple of days later by someone on the issues team.
All so far - fantastic. Warm fuzzies all round.
Three months later, another member of the issues team reverts the commit because
this behaviour is documented elsewhere.
they would ‘prefer it in a different format’.
They’re right that it is documented elsewhere. (In a guide that I never found from looking at the method docs). It has a big red warning box which warns about this behaviour.
To my mind - that underlines the need to document a ‘gotcha’ where users are likely to find it (at the method definition)
As to preferred format - I would suggest that after three months, the correct approach to ‘preferring a different format’ is a PR, rather than a reversion.
Of course - the Rails team absolutely doesn’t have to accept my tiny contributions.
But if they want people to contribute, then I would suggest that this is not a respectful way to treat contributors.
First of all, thank you for sharing your contribution to improving the Rails documentation. We appreciate your efforts.
However, in defense of the Issues team, I’d like to offer a different perspective on the situation.
The Issues team, like many others in open-source, does their work voluntarily alongside their daytime jobs, family lives, and other commitments. This can sometimes result in delays in responding to PRs. It’s also important to recognize that they have the right to reassess their own decisions to maintain the high standards they’ve set for Rails and Rails documentation - even after a PR has been merged. I see the Core, Committers, and Issues team constantly doing this to their own work, too. They have set a high standard for Rails, and they hold themselves, eachother, and other contributors up to these high standards.
As you probably well know, reverting and modifying PRs is a common practice in open-source and can be a valuable learning experience. The Issues team member’s decision to revert your commit was not a dismissal of your contribution but rather an effort to guide you in aligning with the project’s standards.
It’s similar to a rollback of code in production: sometimes it’s more effective to revert and fix the issue properly. This team member could have pushed a fix themselves, but that would have deprived you of the chance to learn and grow as a contributor. This is because they want you to learn and continue contributing- this means that each time you contribute it will get easier and easier.
This level of back-and-forth collaboration and constructive feedback is highly valued among the Issues, Committers, and Core teams - this is how they work together too.
I hope you see this as an invitation to continue contributing. Your involvement is appreciated, and we’d love to see your contributions again if you decide to continue.
I appreciate your response, but I don’t see this as a healthy back and forth.
My contribution was initially guided by a core team member, then approved by an issues member.
Then another issues member (three months later) knows better and hits the revert button.
We’re not talking about some key high-performance code here. We’re talking about whether it is helpful for some documentation at a method site to repeat a warning that is elsewhere in a guide.
So, now it’s a pissing match between one committer who knows better than the other two committers. And as a contributor I’m supposed to learn what will keep all of them happy.
And the net result here is that there is no warning in the method documentation where last week there was one.
Obviously - the Rails team has no obligation to accept help from any contributor.
But I think you’re allowing an arbitrary measure of perfect to get in the way of good - and that’s not a game I have any interest in playing.
Just to be clear, the ‘learn and grow’ you are encouraging me to embrace is that I should learn how to make a documentation contribution better than what was deemed up to scratch by one core member, and one issues member. That’s not a standard I aspire to. Rails docs just are not that important to me. I just want(ed) to make some simple casual contributions which improve the docs where I come across obvious issues.
There might be a culture thing to think about here.
I get that in important core code, an ongoing back and forth to get stuff perfect makes sense.
I don’t think that is a helpful standard in docs. I suspect most people are like me, and have minimal interest in learning ‘the perfect Rails Style’
To me, the standard is something along the lines of
Is is clear & correct
does it broadly meet some style guide
And the standard for accepting/rejecting a pull request would not be ‘is this the perfect style’, but rather ‘does this make the docs better’
Of course, you get to set the bar how you choose - but if you want to encourage more contributions to docs, then it might make sense to think about where you want to set it.
While all of this I’m sure is… beating the proverbial dead horse…
I think my biggest question out of all this would be… Is there some guide to … guide [new] contributors on the preferred place for documenting specific things? Like OP, I look at the API docs WAY more than the Guides. But if non-obvious “gotchas”, foot guns or what have you are supposed to be documented specifically and only in the guides, well, I will make sure to keep two tabs pinned.
It makes little sense to me why the API docs would not point out “assigning a named method callback will REMOVE it from other callbacks”. Or… (cause I’m totally a linguist) “named methods can only be assigned to a single callback”…
Semantically; This API documentation page has a Caveats section. This wouldn’t be considered a caveat?
Alternatively; This is probably something that should print some sort of warning? My code specifically stated one thing, yet by way of design, was made precisely not that. It’s not in the API doc, its not noted in Caveats, how can I trust it at all?
Rails has too many gotchas that require a trip to SO to figure out. The documentation is too often missing a warning in an obvious place.
If this kind of “reversion” editing is common, that would explain why the gotchas remain hidden to many, instead of understood from the start.
I don’t like it, but it would be easier to fix if the culture is willing to change from “writing as tersely as possible about the code” to “saving people headaches whenever possible”.
Just to be clear, the Issues team is open to the warning being in the API docs, just located at the top-level and not repeated multiple times throughout. You can see the comment here.
OP just didn’t want to create another PR, so if anyone wants to, feel free!
Totally agree with OP; this is DRY as the be all end all regardless of side effects.
The bottom line for me is that the core team and issues team have not agreed on what the rules/guidelines are for documentation. Otherwise why would a thoughtfully-considered approval by two people get arbitrarily and abruptly (some might say arrogantly) thrown out by one person three months later?
And when inconsistencies are pointed out the response is essentially “the issues team knows better than you: so sorry you don’t understand.”