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

Rename CrateAction>Notifications to Sound and add an actual Notifications field. #15699

Merged
merged 4 commits into from Oct 26, 2018

Conversation

Projects
None yet
4 participants
@MustaphaTR
Copy link
Member

MustaphaTR commented Oct 12, 2018

Closes #15698.

@pchote pchote added this to the Next + 1 milestone Oct 12, 2018

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:crate-notifications branch from a3e31f4 to bdf0e1f Oct 12, 2018

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:crate-notifications branch from bdf0e1f to 99a24d2 Oct 12, 2018

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:crate-notifications branch from 99a24d2 to 780f688 Oct 12, 2018

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:crate-notifications branch from 780f688 to 56b48df Oct 12, 2018

@abcdefg30
Copy link
Member

abcdefg30 left a comment

Looks good to me otherwise. 👍

get
{
return "*CrateAction: traits now have actual `Notification:` field.\n" +
"New `Sound:` field does what old `Notification:` did.";

This comment has been minimized.

@abcdefg30

abcdefg30 Oct 13, 2018

Member

I'd suggest adding some articles and omitting the colons:

return "'*CrateAction' traits now have an actual `Notification` field.\n" +
	"The new `Sound` field does what the old `Notification` did.";
@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 14, 2018

Updated.

@abcdefg30

This comment has been minimized.

Copy link
Member

abcdefg30 commented Oct 25, 2018

Needs a rebase again...

@MustaphaTR MustaphaTR force-pushed the MustaphaTR:crate-notifications branch from 90308c8 to 70c92e0 Oct 25, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 25, 2018

Rebased.

@pchote

pchote approved these changes Oct 26, 2018

@pchote pchote merged commit 98006d3 into OpenRA:bleed Oct 26, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MustaphaTR MustaphaTR deleted the MustaphaTR:crate-notifications branch Oct 27, 2018

@MustaphaTR

This comment has been minimized.

Copy link
Member

MustaphaTR commented Oct 27, 2018

It seems like i forgot to put the update rule in the UpdatePath.cs.

@pchote

This comment has been minimized.

Copy link
Member

pchote commented Oct 27, 2018

Can you please file a PR to fix that?

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