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.

Sign up for GitHub

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

Jump to bottom

Adding Arabic localization support #2593

Merged
merged 24 commits into from
Nov 25, 2019
Merged

Adding Arabic localization support #2593

merged 24 commits into from
Nov 25, 2019

Conversation

midineo
Copy link
Contributor

@midineo midineo commented Nov 13, 2019 •
edited
Loading

Description

Adding Arabic localization support

Specific Changes

I reviewed many rejected PRs for the Arabic localization, to make sure I fulfilled the needed requirements.

@msftclas
Copy link

msftclas commented Nov 13, 2019 •
edited
Loading

CLA assistant check
All CLA requirements met.

@coveralls
Copy link

coveralls commented Nov 13, 2019 •
edited
Loading

Coverage Status

Coverage decreased (-0.6%) to 63.554% when pulling fa04915 on midineo:master into 2d123c5 on microsoft:master.

@midineo
Copy link
Contributor Author

midineo commented Nov 20, 2019

Hello Team,

It's 7 days since I created PR, should it take that long or May I missed something.
Please let me know if there is anything pending on me.

Thanks,
Midineo

corinagum
corinagum suggested changes Nov 20, 2019
View reviewed changes
Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

Thanks for your patience @midineo!! @tdurnford and I left some comments. Please address these and also add an entry to CHANGELOG.md (Under the Unreleased Added section), and we are excited to get this approved!

@midineo
Copy link
Contributor Author

midineo commented Nov 21, 2019

Thanks @corinagum, I will apply them shortly :)

midineo and others added 3 commits November 21, 2019 10:22
@midineo @tdurnford
Update packages/component/src/Localization/Localize.js
6a7d302 
Co-Authored-By: TJ Durnford <tjdford@gmail.com>
@midineo @tdurnford
Update packages/component/src/Localization/Localize.js
efa2a30 
Co-Authored-By: TJ Durnford <tjdford@gmail.com>
@midineo @corinagum
Update packages/component/src/Localization/ar-EG.js
9d02f91 
Co-Authored-By: Corina <14900841+corinagum@users.noreply.github.com>
@midineo
Copy link
Contributor Author

midineo commented Nov 21, 2019

Review Comments applied, however, I can't figure out why BotFramework-WebChat-CI-PR is failing

@corinagum
Copy link
Contributor

Thanks @midineo, sometimes the tests have timeouts. I just restarted the build and I'll keep an eye on it for today. :)

corinagum
corinagum reviewed Nov 21, 2019
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
corinagum
corinagum suggested changes Nov 21, 2019
View reviewed changes
Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

Hi @midineo, just a couple more things and then I'll babysit the build. Thank you!

@midineo
Copy link
Contributor Author

midineo commented Nov 24, 2019

Hi @midineo, just a couple more things and then I'll babysit the build. Thank you!

Hi @corinagum, Sorry for my late reply, I applied your comments. Thanks for your efforts.

corinagum
corinagum approved these changes Nov 25, 2019
View reviewed changes
Copy link
Contributor

@corinagum corinagum left a comment

Choose a reason for hiding this comment

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

This looks good to me! We're having some problems with the build, and I am investigating those now. Hopefully they'll resolve on their own. Please stand by - hope to get this merged in asap!

@corinagum corinagum merged commit d4ddd6e into microsoft:master Nov 25, 2019
@compulim compulim mentioned this pull request Dec 10, 2019
Closed
73 tasks
@corinagum corinagum mentioned this pull request Dec 13, 2019
Closed
73 tasks
@corinagum corinagum mentioned this pull request Feb 7, 2020
Merged
52 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdurnford tdurnford tdurnford left review comments

@corinagum corinagum corinagum approved these changes

@a-b-r-o-w-n a-b-r-o-w-n Awaiting requested review from a-b-r-o-w-n a-b-r-o-w-n is a code owner

@compulim compulim Awaiting requested review from compulim compulim is a code owner

@cwhitten cwhitten Awaiting requested review from cwhitten cwhitten is a code owner

@tonyanziano tonyanziano Awaiting requested review from tonyanziano

Assignees
No one assigned
Labels
Pre-release: Need Manual Testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@midineo @msftclas @coveralls @corinagum @tdurnford