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

Css classes refactor #171

Merged
merged 29 commits into from
Oct 20, 2017
Merged

Css classes refactor #171

merged 29 commits into from
Oct 20, 2017

Conversation

robertcoopercode
Copy link
Collaborator

@robertcoopercode robertcoopercode commented Oct 20, 2017

Fixes #114 and #162


This change is Reviewable

@vinitkumar
Copy link
Owner

@Engineering-Robert Would really appreciate if you can keep the PRs smaller. Smaller PRs are easier to review and more possible to spot bugs that way. Also, different issues deserve different PRs as they keep the context clear.

Another thing, I am travelling today and tomorrow so most probably be online only on Sunday.

@robertcoopercode
Copy link
Collaborator Author

Ya, this one got quite lengthy, my apologies. However, I did want to get all this work within this pull request since I wanted you to merge all of this today. The alternative would have been to do half the work, make a PR, wait for you to merge, do the other work, make another PR, have you merge the PR.
I see two possible solutions to the above problem, and that would be to either merge the smaller PRs myself, or I can make branches on a forked repository, merge PRs into master on the fork along with making PRs from my forked branch to the original master branch.

@robertcoopercode robertcoopercode merged commit 622cb1c into master Oct 20, 2017
@robertcoopercode
Copy link
Collaborator Author

I did not mean to merge this. I thought I was on my forked repository! I'm trying to revert it right now.

@vinitkumar
Copy link
Owner

vinitkumar commented Oct 20, 2017 via email

@robertcoopercode robertcoopercode deleted the css-classes-refactor branch October 20, 2017 15:24
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.

2 participants