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

Add condition for merge straight #183

Merged
merged 4 commits into from
Oct 27, 2017
Merged

Add condition for merge straight #183

merged 4 commits into from
Oct 27, 2017

Conversation

lyzidiamond
Copy link
Contributor

@lyzidiamond lyzidiamond commented Oct 24, 2017

When the modifier is straight on a merge instruction, instructions should say "merge onto This Highway" instead of "merge straight onto This Highway" because "merge straight" doesn't make any sense.

  • Add the case
  • Add changelog entry
  • Review

Copy link

@allierowan allierowan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! so many releases today...

@lyzidiamond
Copy link
Contributor Author

I might hold off on this release for now... I want @mcwhittemore's eyes on the default part of this addition.

To clarify following questions from @bsudekum, this is adding special handling for a merge maneuver with a straight modifier.

Copy link
Contributor

@mcwhittemore mcwhittemore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather then filling in the new merge.straight string with english, we should use the merge.default string from each language and replace the {modifier} with the language's straight word.

This will let translators make a similar change later without effecting the text now.

@allierowan
Copy link

fyi @lyzidiamond I incorporated @mcwhittemore's comment :)

@mcwhittemore can I get another 👁 ?

@mcwhittemore mcwhittemore merged commit 49e3527 into master Oct 27, 2017
@mcwhittemore mcwhittemore deleted the merge-straight branch October 27, 2017 20:29
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.

3 participants