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

Updated L.Control.Layers initializer to class factory syntax #867

Closed
wants to merge 2 commits into from
Closed

Updated L.Control.Layers initializer to class factory syntax #867

wants to merge 2 commits into from

Conversation

OzunaEddie
Copy link
Contributor

@OzunaEddie OzunaEddie commented Jul 19, 2019

Fixes #860 (<=== Add issue number here)

Make sure these boxes are checked before your pull request (PR) is ready to be reviewed and merged. Thanks!

  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/mapknitter-reviewers for help, in a comment below

We're happy to help you get this ready -- don't be afraid to ask for help, and don't be discouraged if your tests fail at first!

If tests do fail, click on the red X to learn why by reading the logs.

Please be sure you've reviewed our contribution guidelines at https://publiclab.org/contributing-to-public-lab-software

Thanks!

@welcome
Copy link

welcome bot commented Jul 19, 2019

Thanks for opening this pull request! Dangerbot will test out your code and reply in a bit with some pointers and requests.
There may be some errors, but don't worry! We're here to help! 👍🎉😄

@codeclimate
Copy link

codeclimate bot commented Jul 19, 2019

Code Climate has analyzed commit e2eaa85 and detected 0 issues on this pull request.

View more on Code Climate.

@codecov

This comment has been minimized.

Copy link
Member

@kaustubh-nair kaustubh-nair 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!
Thanks @OzunaEdd

@sashadev-sky
Copy link
Member

@OzunaEdd I think this is an issue with a configuration setting we're still working on, but I couldn't check it out on your fork maybe because you made the PR from your main branch? Can you try instead moving it to a feature branch?

You can do:

  1. $ git reset --hard HEAD@{0} - this is going to reset your main branch back to before you ever made changes on it
  2. $ git checkout -b <new-branch-name> - create new branch and go to it
  3. $ git branch - to confirm you are there
  4. $ git cherry-pick e2eaa85d8 - move your last commit that was on main to current branch
  5. reupdate the capitalization of L.Control.Layers that you did in the 1st commit (or you can cherry-pick that commit too but there were some automatic syntax updates from your code editor so better to just redo it)
  6. $ git put origin <new-branch-name>
  7. I think that you will need to open a new PR for that because its on a new branch

Also when you're done you can make sure main is set back to normal in your remote by returning to the main branch ($ git checkout main) and running $ git push -f origin main

@sashadev-sky
Copy link
Member

closing as this was resolved in #869! Thanks again @OzunaEdd :)

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.

Update L.Control.Layers initializer to class factory syntax
3 participants