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

Option for changing color of the Rotating texts #9 #29

Merged
merged 10 commits into from Jun 18, 2020
Merged

Option for changing color of the Rotating texts #9 #29

merged 10 commits into from Jun 18, 2020

Conversation

Anu-123-gif
Copy link
Contributor

@Anu-123-gif Anu-123-gif commented Jun 8, 2020

An array of integers representing colors chosen by the user is added to a rotatable and the words are displayed accordingly.

Please review my code @CoderMayhem

@Arnesh07
Copy link
Member

Arnesh07 commented Jun 8, 2020

Hi @Anu-123-gif , Thanks for your contribution!
Please assign reviewers to this pull request. This way, they will get a notification regarding the same.
Thanks!

Copy link
Collaborator

@CoderMayhem CoderMayhem left a comment

Choose a reason for hiding this comment

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

Your present implementation of the feature is working, but I think the code can be improved in order to prevent any errors when other developers are using the library. Take a look at the changes I have requested.

I really appreciate your efforts.
Happy Coding!

Copy link
Collaborator

@CoderMayhem CoderMayhem left a comment

Choose a reason for hiding this comment

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

Make the reviewed changes.

Copy link
Collaborator

@CoderMayhem CoderMayhem left a comment

Choose a reason for hiding this comment

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

Good job with the changes! Kindly reply to the suggestion I have made.

Copy link
Collaborator

@CoderMayhem CoderMayhem left a comment

Choose a reason for hiding this comment

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

Good Job @Anu-123-gif . @dev-ritik need your feedback on the code before we can merge it. I have tested it, its working fine.

@dev-ritik
Copy link
Member

LGTM! 🎉

@dev-ritik
Copy link
Member

dev-ritik commented Jun 18, 2020

@Anu-123-gif Resolve these conflicts, and we are good to go.
(Fetch the master -> rebase on origin/master -> resolve the conflicts -> test if things work -> force push your branch)

You can resolve in the web editor quickly but its recommended that you do it via terminal.

@Anu-123-gif
Copy link
Contributor Author

Anu-123-gif commented Jun 18, 2020

@Anu-123-gif Resolve these conflicts, and we are good to go.
(Fetch the master -> rebase on origin/master -> resolve the conflicts -> test if things work -> force push your branch)

You can resolve in the web editor quickly but its recommended that you do it via terminal.

@Anu-123-gif Resolve these conflicts, and we are good to go.
(Fetch the master -> rebase on origin/master -> resolve the conflicts -> test if things work -> force push your branch)

You can resolve in the web editor quickly but its recommended that you do it via terminal.

@dev-ritik I have resolved conflicts , thanks !

@dev-ritik dev-ritik merged commit 9d8a330 into mdgspace:master Jun 18, 2020
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.

None yet

4 participants