WEBVTT

NOTE Created by CaptionSync from Automatic Sync Technologies www.automaticsync.com

00:00:01.016 --> 00:00:03.926 align:middle
The first thing to notice is the base branch.

00:00:04.646 --> 00:00:06.296 align:middle
This is super important.

00:00:06.826 --> 00:00:11.626 align:middle
Because we created our branch off of
master, this base branch must be master.

00:00:12.716 --> 00:00:18.396 align:middle
If we created the branch off of 3.4,
then, of course, this should be 3.4.

00:00:19.336 --> 00:00:22.886 align:middle
One easy way to be sure you have
things setup correctly is to check

00:00:22.886 --> 00:00:26.426 align:middle
that you only see the commits
down here that you expect.

00:00:26.426 --> 00:00:32.366 align:middle
If you mess up the base branch, you'll probably
see a bunch of extra commits and changes.

00:00:33.916 --> 00:00:37.766 align:middle
The pull request description comes
with a nice template to get us started.

00:00:37.766 --> 00:00:43.616 align:middle
For the branch, because this is a new
feature, we do want the master branch.

00:00:44.246 --> 00:00:47.626 align:middle
This is not a bug fix and this is a new feature.

00:00:48.576 --> 00:00:54.136 align:middle
Oh, and if this is a new feature,
apparently, we should update the CHANGELOG.

00:00:54.486 --> 00:00:56.396 align:middle
I totally forgot about that!

00:00:57.516 --> 00:00:58.456 align:middle
Go back to your editor.

00:00:59.006 --> 00:01:02.706 align:middle
Then, inside whatever bundle or
component you're working on -

00:01:02.896 --> 00:01:07.516 align:middle
so SecurityBundle for us -
find the CHANGELOG.md file.

00:01:08.806 --> 00:01:15.916 align:middle
If our new feature is accepted, it will be in
the next minor version, which is 4.2 right now.

00:01:16.826 --> 00:01:19.436 align:middle
You can already see a few
new features listed there.

00:01:20.016 --> 00:01:28.036 align:middle
Progress! Let's add our new feature: describe
what we introduced & why it's useful.

00:01:30.276 --> 00:01:34.096 align:middle
To add this, we could just make a second commit.

00:01:34.586 --> 00:01:36.836 align:middle
And that would be totally fine.

00:01:37.366 --> 00:01:41.716 align:middle
In fact, if you're making significant
changes to the pull request,

00:01:42.056 --> 00:01:48.586 align:middle
making new commits is a good idea: it will
help people see how your pull request evolves

00:01:48.586 --> 00:01:49.116 align:middle
over time.

00:01:49.716 --> 00:01:56.016 align:middle
But, in this case, because the changes are
so simple, run: git add -u to add everything.

00:01:56.516 --> 00:02:01.766 align:middle
Then: git commit -- amend
That will add these changes

00:02:01.766 --> 00:02:04.436 align:middle
to my previous commit to keep things clean.

00:02:05.486 --> 00:02:13.936 align:middle
Push with: git push weaverryan
target-path-helper -- force Perfect!

00:02:14.446 --> 00:02:15.686 align:middle
Head back to the pull request.

00:02:16.276 --> 00:02:19.296 align:middle
We did not introduce any
backwards compatibility breaks,

00:02:19.786 --> 00:02:26.116 align:middle
we didn't deprecate any features
and yes, the tests should pass.

00:02:27.116 --> 00:02:30.266 align:middle
A lot of these are just reminders
to think about things.

00:02:30.266 --> 00:02:35.136 align:middle
After we submit the PR, we'll
see for sure if the tests pass.

00:02:35.136 --> 00:02:45.436 align:middle
For fixed tickets, there isn't always a fixed
ticket, but there is in this case: #27835.

00:02:45.436 --> 00:02:50.526 align:middle
Oh, and whenever you add a new feature, you
need to create a documentation pull request.

00:02:51.146 --> 00:02:52.596 align:middle
I'll put TODO for now.

00:02:53.106 --> 00:02:55.736 align:middle
But we are going to do this soon.

00:02:55.736 --> 00:03:00.906 align:middle
Finally, at the bottom, it's
our job to put a short README

00:03:00.946 --> 00:03:04.026 align:middle
so that other people can
understand how our feature works.

00:03:04.686 --> 00:03:09.976 align:middle
Showing some code examples is best - but
because the code behind this is pretty simple,

00:03:10.376 --> 00:03:14.166 align:middle
what I really want to do is
describe why this is needed.

00:03:16.106 --> 00:03:17.266 align:middle
And... we're ready!

00:03:17.716 --> 00:03:19.406 align:middle
Create that pull request!

00:03:22.946 --> 00:03:24.816 align:middle
Boom! Great work people!

00:03:25.516 --> 00:03:27.396 align:middle
Will this pull request be accepted?

00:03:27.916 --> 00:03:28.836 align:middle
Who knows?

00:03:29.186 --> 00:03:35.356 align:middle
But, we did good work, wrote tests and
clearly stated why we created this feature.

00:03:35.856 --> 00:03:38.806 align:middle
We rock! Until...

00:03:38.806 --> 00:03:42.456 align:middle
yep! We immediately see failures at the bottom!

00:03:43.616 --> 00:03:46.186 align:middle
Two things happen when you
create a pull request.

00:03:46.296 --> 00:03:54.306 align:middle
First, Symfony's continuous integration system
starts running the tests: Travis CI runs tests

00:03:54.306 --> 00:03:57.486 align:middle
on Linux & AppVeyor runs them on Windows.

00:03:58.556 --> 00:04:01.196 align:middle
Click the details for Travis
- it's pretty awesome.

00:04:04.586 --> 00:04:10.206 align:middle
It executes the tests on multiple
versions of PHP and uses different flags

00:04:10.316 --> 00:04:15.776 align:middle
to use both the newest version of Symfony's
dependencies and the oldest allowed versions.

00:04:16.446 --> 00:04:17.856 align:middle
We'll let this keep doing its thing.

00:04:20.506 --> 00:04:23.636 align:middle
The second thing that happens
after creating a pull request is

00:04:23.636 --> 00:04:25.746 align:middle
that you are visited by the famous...

00:04:26.036 --> 00:04:34.246 align:middle
fabbot! Fabbot automatically checks your
pull request for coding standards violations.

00:04:35.306 --> 00:04:37.666 align:middle
Apparently we have two problems!

00:04:37.986 --> 00:04:45.666 align:middle
But, here's the best part: copy that curl
statement, find your terminal, and paste!

00:04:47.176 --> 00:04:52.246 align:middle
Run: git diff Cool!

00:04:52.716 --> 00:04:58.446 align:middle
This automatically fixed the two whitespace
issues that violated Symfony's coding standards.

00:04:59.376 --> 00:05:05.886 align:middle
This is why I don't worry too much about coding
standards until now: fabbot will help us.

00:05:06.346 --> 00:05:11.446 align:middle
And because these changes aren't important,
just like before, let's amend our commit:

00:05:12.006 --> 00:05:21.536 align:middle
git add -ugit commit -- amend Push that with:
git push weaverryan target-path-helper --

00:05:21.536 --> 00:05:27.206 align:middle
force Ok, go back to the pull request.

00:05:28.636 --> 00:05:30.096 align:middle
Refresh and...

00:05:30.646 --> 00:05:33.476 align:middle
yea! Fabbot is happy!

00:05:34.226 --> 00:05:35.896 align:middle
So... what now?

00:05:35.896 --> 00:05:39.186 align:middle
First, wait to see if the tests pass.

00:05:39.766 --> 00:05:44.936 align:middle
If they don't, yea, you'll need to see if
our changes caused some unexpected bug.

00:05:45.526 --> 00:05:48.356 align:middle
And second, wait for community feedback!

00:05:48.866 --> 00:05:51.776 align:middle
Sometimes, feedback can be
direct and to the point.

00:05:52.406 --> 00:05:56.636 align:middle
We're developers, so we like to look
at all the smallest technical details.

00:05:57.386 --> 00:06:03.926 align:middle
Don't take this personally: feedback is meant
to be constructive - we're all on the same side.

00:06:04.716 --> 00:06:09.906 align:middle
And, yea, you'll almost definitely
need to make at least some changes.

00:06:10.416 --> 00:06:16.956 align:middle
Heck, I rarely make a pull request that doesn't
need significant changes after some feedback.

00:06:17.566 --> 00:06:19.026 align:middle
It's awesome!

00:06:19.986 --> 00:06:24.086 align:middle
Usually, someone thinks of
a way better implementation.

00:06:24.716 --> 00:06:29.746 align:middle
When that happens, you know the
drill: make the changes, commit, push,

00:06:30.006 --> 00:06:32.286 align:middle
and check fabbot and the tests again.

00:06:33.516 --> 00:06:38.956 align:middle
Oh, and don't worry too much about doing
the git commit -- amend thing or rebasing.

00:06:39.556 --> 00:06:42.366 align:middle
It's totally ok to have multiple commits.

00:06:42.806 --> 00:06:48.976 align:middle
And also, when someone merges your pull request,
they use a tool that makes it really easy

00:06:48.976 --> 00:06:53.586 align:middle
to squash all of your commits down
into 1 commit, if they want to.

00:06:54.286 --> 00:06:56.806 align:middle
That's not something you need to worry about.

00:06:58.016 --> 00:07:01.576 align:middle
Next: we haven't created our
documentation pull request yet.

00:07:02.286 --> 00:07:03.376 align:middle
Time to do that!