Update themes-settings.html and strings.js #8715

Merged
merged 5 commits into from Aug 20, 2014

Projects

None yet

5 participants

@Denisov21
Contributor

Fix #8712 (part 1 of 2)

@redmunds
Contributor

@Denisov21 Please put all of you files in a single pull request. I tried explain this in the Code Review section in this article, but maybe I wasn't clear enough. In you particular case, do this:

  1. After submitting your first pull request (this one), look at the top where it says: "Denisov21 wants to merge 2 commits into adobe:master from Denisov21:patch-12 "
  2. Go to your github fork of brackets page: github.com/Denisov21/brackets
  3. Click on the Branches Tab
  4. Click on the link to branch patch-12
  5. All changes from there will be to this same pull request.
@redmunds
Contributor

@Denisov21 FYI, I updated the NLS README file with this section: Updating Existing Branch and Pull Request to make this more clear. Let me know if you have any suggestions for improvements.

@peterflynn
Member

Yep. We can't merge this without the changes in #8716 being part of the same merge. There's no reason for this to be split into 2 PRs (or even 2 separate commits, really).

Denisov21 added some commits Aug 11, 2014
@Denisov21 Denisov21 Update strings.js 852384d
@Denisov21 Denisov21 Update strings.js
Changes made -- ready for another review
e9577bd
@Denisov21
Contributor

@redmunds Sorry for the inconvenience, it was my first PR with two different files. Your explanation is very good!
@redmunds @peterflynn Changes made -- ready for another review

@Denisov21 Denisov21 changed the title from Update themes-settings.html to Update themes-settings.html and strings.js Aug 12, 2014
@MiguelCastillo MiguelCastillo commented on an outdated diff Aug 20, 2014
src/htmlContent/themes-settings.html
@@ -1,6 +1,6 @@
<div class="theme-settings modal">
<div class="modal-header">
- <h1 class="dialog-title">Themes Settings</h1>
+ <h1 class="dialog-title">{{Strings.THEME_SETTINGS}}</h1>
@MiguelCastillo
MiguelCastillo Aug 20, 2014 Member

Strings.THEME_SETTINGS does not match what you have defined in strings.js.

@Denisov21
Contributor

@MiguelCastillo Changes made -- ready for another review!

@MiguelCastillo
Member

This one looks good to be merged.

@ingorichter ingorichter self-assigned this Aug 20, 2014
@ingorichter ingorichter merged commit d11f015 into adobe:master Aug 20, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details
@ingorichter
Contributor

Thanks @Denisov21!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment