Skip to content

Commit 189e632

Browse files
committed
Merge branch '2.7' into 2.8
* 2.7: Update forms.rst Add missing colon after protocol in link [Contributing] Reviewing an issue/pull-request (Giving constructive criticism) minor #9328 Markup in Config Validation example (ProgMiner) Fix static method call Explaining the "one-to-many" case more explicitly
2 parents 6bea887 + fa65333 commit 189e632

File tree

8 files changed

+196
-13
lines changed

8 files changed

+196
-13
lines changed

components/config/definition.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -729,7 +729,7 @@ The builder is used for adding advanced validation rules to node definitions, li
729729
->scalarNode('driver')
730730
->isRequired()
731731
->validate()
732-
->ifNotInArray(array('mysql', 'sqlite', 'mssql'))
732+
->ifNotInArray(array('mysql', 'sqlite', 'mssql'))
733733
->thenInvalid('Invalid database driver %s')
734734
->end()
735735
->end()

components/console/helpers/progressbar.rst

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -289,8 +289,9 @@ Progress bars define a placeholder called ``message`` to display arbitrary
289289
messages. However, none of the built-in formats include that placeholder, so
290290
before displaying these messages, you must define your own custom format::
291291

292+
ProgressBar::setFormatDefinition('custom', ' %current%/%max% -- %message%');
293+
292294
$progressBar = new ProgressBar($output, 100);
293-
$progressBar->setFormatDefinition('custom', ' %current%/%max% -- %message%');
294295
$progressBar->setFormat('custom');
295296

296297
Now, use the ``setMessage()`` method to set the value of the ``%message%``
@@ -308,8 +309,9 @@ placeholder before displaying the progress bar::
308309
Messages can be combined with custom placeholders too. In this example, the
309310
progress bar uses the ``%message%`` and ``%filename%`` placeholders::
310311

312+
ProgressBar::setFormatDefinition('custom', ' %current%/%max% -- %message% (%filename%)');
313+
311314
$progressBar = new ProgressBar($output, 100);
312-
$progressBar->setFormatDefinition('custom', ' %current%/%max% -- %message% (%filename%)');
313315
$progressBar->setFormat('custom');
314316

315317
The ``setMessage()`` method accepts a second optional argument to set the value

contributing/community/index.rst

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,5 +5,6 @@ Community
55
:maxdepth: 2
66

77
releases
8+
review-comments
89
reviews
910
other
Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
Respectful Review Comments
2+
==========================
3+
4+
:doc:`Reviewing issues and pull requests </contributing/community/reviews>`
5+
is a great way to get started with contributing to the Symfony community.
6+
Anyone can do it! But before you give a comment, take a step back and think,
7+
is what you are about to say actually what you intend?
8+
9+
Communicating over the Internet with nothing but text can pose a
10+
big challenge, especially if you remember that the Symfony community
11+
is world-wide and is composed of a wide variety of people with differing
12+
ideas and opinions.
13+
14+
Not everyone speaks English or is able to use a keyboard. Some might
15+
have dyslexia or similar conditions that affect their writing.
16+
17+
Not to mention that some might have a bad experience from previous
18+
contributions (to other projects).
19+
20+
You're not alone in this. This guide will try to help you write
21+
constructive, respectful and helpful reviews and replies.
22+
23+
.. tip::
24+
25+
This guide is not about lecturing you to "conform" or give-up
26+
your ideas and opinions but helping you to better communicate,
27+
prevent possible confusion, and keeping the Symfony community a
28+
welcoming place for everyone. **You are free to disagree with
29+
someone's opinions, just don't be disrespectful.**
30+
31+
First of, accept that many programming decisions are opinions.
32+
Discuss trade offs, which you prefer, and reach a resolution quickly.
33+
It's not about being right or wrong, but using what works.
34+
35+
Tone of Voice
36+
-------------
37+
38+
We don't expect you to be completely formal, or to even write error-free
39+
English. Just remember this: don't swear, and be respectful to others.
40+
41+
Don't reply in anger or with an aggressive tone. You're angry, we understand
42+
that, but swearing/cursing and name calling doesn't really encourage anyone to
43+
help you. Take a deep breath, count to 10 and try to *clearly* explain what problems
44+
you encounter.
45+
46+
Gender-neutral Pronouns
47+
-----------------------
48+
49+
While not "formally" required, it's better to use gender-neutral pronouns.
50+
Unless someone "indicated" their pronouns, use "they", "them" instead of
51+
"he", "she", "his", "hers", "his/hers", "he/she", etc.
52+
53+
Try to avoid using wording that may be considered excluding and needlessly gendered,
54+
like for example words that have a male base. For example we recommend to use other
55+
words like "folks", "team", "everyone" in place of "guys".
56+
57+
Giving Positive Feedback
58+
------------------------
59+
60+
While reviewing issues and pull requests you may run into some suggestions
61+
(including patches) that don't reflect your ideas, are not good, or downright wrong.
62+
63+
Now, when you prepare your comment, consider the amount of work and time the author
64+
has spent on their idea and how your response would make them feel.
65+
66+
Did you correctly understand their intention? Or are you making assumptions?
67+
Whatever your response, be explicit. Remember people don't always understand your
68+
intentions online.
69+
70+
Avoid using terms that could be seen as referring to personal traits ("dumb", "stupid").
71+
Assume everyone is intelligent and well-meaning.
72+
73+
.. tip::
74+
75+
Good questions avoid judgement and avoid assumptions about the author's perspective.
76+
77+
Maybe you can ask for clarification? Suggest an alternative?
78+
Or provide a simple explanation *why* you disagree with their proposal.
79+
80+
* ``This looks wrong. Are you sure it's correct?`` (eg. typo/syntax error)
81+
82+
* ``What do you think of "RequestFactory" instead of RequestCreator?``
83+
84+
Even if something *is* really wrong or "a bad idea", stay respectful and
85+
don't get into endless you-are-wrong discussions or "flame wars".
86+
87+
Don't use hyperbole ("always", "never", "endlessly", "nothing", "worst", "horrible", "terrible").
88+
89+
**Don't:** *"I don't like how you wrote this code"* - there is no clear explanation why you
90+
don't like how it's written.
91+
92+
**Better:** *"I find it hard to read this code as there many nested if statements, can you make it more
93+
readable? By encapsulating some of it's details or maybe adding some comments to explain the overall logic."* -
94+
You explain why you find the code hard to read *and* give some suggestions for improvement.
95+
96+
If a piece of code is in fact wrong, explain why:
97+
98+
* ``This code doesn't comply with Symfony's CS rules. Please see [...] for details``.
99+
100+
* ``Symfony 3 still uses PHP 5 and doesn't allow the usage scalar type-hints.``.
101+
102+
* ``I think the code is less readable now`` - careful here, be sure explain why you think
103+
the code is less readable, and maybe give some suggestions?
104+
105+
**Examples of valid reasons to reject:**
106+
107+
* We tried that in the past (link to the relevant PR) but we needed to revert it for XXX reason.
108+
109+
* That change would introduce too many merge conflicts when merging up Symfony branches.
110+
In the past we've always rejected changes like this.
111+
112+
* I profiled this change and it hurts performance significantly (if you don't profile, it's an opinion, so we can ignore)
113+
114+
* Code doesn't match Symfony's CS rules (e.g. ``use array()`` instead of ``[]``)
115+
116+
* We only provide integration with very popular projects (e.g. we integrate Bootstrap but not your own CSS framework)
117+
118+
* This would require adding lots of code and making lots of changes for a feature that doesn't look so important.
119+
That could hurt maintaining in the future.
120+
121+
Asking for Changes
122+
------------------
123+
124+
Rarely something is perfect from the start, while the code itself is good.
125+
It may not be optimal or conform the Symfony coding style.
126+
127+
Again, understand the author already spent time on the issue and asking
128+
for (small) changes may be misinterpreted or seen as a personal attack.
129+
130+
Be thankful for their work (so far), stay positive and really help them
131+
to make the contribution a great one. *Especially if they are a first
132+
time contributor.*
133+
134+
Use words like "Please", "Thank you" and "Could you" instead of making demands;
135+
136+
* "Thank you for your work so far. I left some suggestions for improvement
137+
to make the code more readable."
138+
139+
* "Your code contains some coding-style problems, can you fix these before
140+
we merge? Thank you"
141+
142+
* "Please use 4 spaces instead of tabs", "This needs be on the previous line";
143+
144+
During a pull request review you can usually leave more then one comment,
145+
you don't have to use "Please" all the time. But it wouldn't hurt.
146+
147+
It may not seem like much, but saying "Thank you" does make others feel
148+
more welcome.
149+
150+
Using Humor
151+
-----------
152+
153+
In short: Extreme misbehavior will not be tolerated and may even get you banned;
154+
Keep it real and friendly.
155+
156+
**Don't use sarcasm for a serious topic, that's not something that belongs
157+
to the Symfony community.** And don't marginalize someone's problems;
158+
``Well I guess that's not supposed to happen? 😆``.
159+
160+
Even if someone's explanation is "inviting to joke about it", it's a real
161+
problem to them. Making jokes about this doesn't help with solving their
162+
problem and only makes them *feel stupid*. Instead try to discover what
163+
the problem is really about.
164+
165+
Final Words
166+
-----------
167+
168+
Don't feel bad if you "failed" to follow these tips. As long as your
169+
intentions were good and you didn't really offended or insult anyone;
170+
you can explain you misunderstood, you didn't meant to marginalize or
171+
simply failed.
172+
173+
But don't say it "just because", if your apology is not really meant
174+
you *will* lose credibility and respect from other developers.
175+
176+
*Do unto others as you would have them do unto you.*

contributing/documentation/overview.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ forked repository and ``improve_install_article`` is the name of the branch you
144144
created previously.
145145

146146
**Step 7.** Everything is now ready to initiate a **pull request**. Go to your
147-
forked repository at ``https//github.com/YOUR-GITHUB-USERNAME/symfony-docs``
147+
forked repository at ``https://github.com/YOUR-GITHUB-USERNAME/symfony-docs``
148148
and click on the **Pull Requests** link located in the sidebar.
149149

150150
Then, click on the big **New pull request** button. As GitHub cannot guess the

contributing/map.rst.inc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@
2323
* **Community**
2424

2525
* :doc:`Release Process </contributing/community/releases>`
26+
* :doc:`Respectful Review comments </contributing/community/review-comments>`
2627
* :doc:`Community Reviews </contributing/community/reviews>`
2728
* :doc:`Other Resources </contributing/community/other>`

form/form_collections.rst

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -552,15 +552,19 @@ you will learn about next!).
552552

553553
// src/AppBundle/Entity/Task.php
554554

555-
// ...
556555
public function addTag(Tag $tag)
557556
{
557+
// for a many-to-many association:
558558
$tag->addTask($this);
559559

560+
// for a many-to-one association:
561+
$tag->setTask($this);
562+
560563
$this->tags->add($tag);
561564
}
562565

563-
Inside ``Tag``, just make sure you have an ``addTask()`` method::
566+
If you're going for ``addTask()``, just make sure you have an appropriate method
567+
that looks something like this::
564568

565569
// src/AppBundle/Entity/Tag.php
566570

@@ -572,9 +576,6 @@ you will learn about next!).
572576
}
573577
}
574578

575-
If you have a one-to-many relationship, then the workaround is similar,
576-
except that you can simply call ``setTask()`` from inside ``addTag()``.
577-
578579
.. _form-collections-remove:
579580

580581
Allowing Tags to be Removed

forms.rst

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -437,14 +437,16 @@ Validation is a very powerful feature of Symfony and has its own
437437
.. code-block:: html+twig
438438

439439
{# app/Resources/views/default/new.html.twig #}
440-
{{ form(form, {'attr': {'novalidate': 'novalidate'}}) }}
440+
{{ form_start(form, {'attr': {'novalidate': 'novalidate'}}) }}
441+
{{ form_widget(form) }}
442+
{{ form_end(form) }}
441443

442444
.. code-block:: html+php
443445

444446
<!-- app/Resources/views/default/new.html.php -->
445-
<?php echo $view['form']->form($form, array(
446-
'attr' => array('novalidate' => 'novalidate'),
447-
)) ?>
447+
<?php echo $view['form']->start($form, array('attr' => array('novalidate' => 'novalidate') ?>
448+
<?php echo $view['form']->widget($form) ?>
449+
<?php echo $view['form']->end($form) ?>
448450

449451
.. index::
450452
single: Forms; Built-in field types

0 commit comments

Comments
 (0)