-
Notifications
You must be signed in to change notification settings - Fork 282
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
Update L.DomUtil method creation to ES6 syntax #629
Comments
assigning @olunusib! |
@sashadev-sky review pull request |
Can I still contribute to the issue? @sashadev-sky |
@roshan0708 it's available now, if you're still interested I will assign you! |
I am interested @sashadev-sky... |
@McZenith ok since we're not sure @roshan0708 still wants to contribute (been a few months) im assigning you :) |
Thank you... I'm exited |
Hi! I initially started working on the issue with a wrong start command... I did npm install instead of npm run set up so I had to delete the first PR and now that I seem to have get the set up right I am still getting another error without even touching the code at all... This is what I got from npm run setup, I need help ... Finished in 46.392 secs / 2.519 secs @ 13:25:50 GMT+0100 (West Africa Standard Time) Warning: Task "karma:development:start" failed. Use --force to continue. Aborted due to warnings. npm ERR! A complete log of this run can be found in: npm ERR! A complete log of this run can be found in: |
@McZenith Feel free to open up a new PR. Also, disregard what I said earlier about the For future knowledge, if you commit something by mistake there are many different ways to undo the changes and it is almost never the right answer to close the PR and start a new one from scratch. So instead just ask on the PR for help. |
@McZenith also are you running the commands locally on your computer or through an online platform? |
Hi @sashadev-sky I'm running it locally. Thanks for the comment I really appreciate I'm actually new to open source contribution. |
@McZenith I just merged an update into the |
Thank you @sashadev-sky! I'm excited, more than ever. |
First Time?
This is a first-timers-only issue. This means we've worked to make it more legible to folks who either haven't contributed to our codebase before, or even folks who haven't contributed to open source before.
If that's you, we're interested in helping you take the first step and can answer questions and help you out as you do. Note that we're especially interested in contributions from people from groups underrepresented in free and open source software!
We know that the process of creating a pull request is the biggest barrier for new contributors. This issue is for you 💝
If you have contributed before, consider leaving this one for someone new, and looking through our general help wanted issues. Thanks!
🤔 What you will need to know.
Nothing. This issue is meant to welcome you to Open Source :) We are happy to walk you through the process.
The problem
We just added support for ES6 in our library, and would like to start transitioning our code to use this syntax.
Let's update the method creation syntax in the
src/util/DomUtil.js
file from ES5 to ES6.Solution
Where to find the relevant lines of code:
For the 5 functions in
src/util/DomUtil.js
, remove thefunction
keyword and attach the method arguments to the method name instead.Here is an example of how this will look for a different method. These ones should follow suit:
ES5
ES6
Thanks!!
Step by Step
commit your changes to your branch and start a pull request (see contributing to Public Lab software) but mark it as "in progress" if you have questions or if you haven't finished
Please keep us updated
💬⏰ - We encourage contributors to be respectful to the community and provide an update within a week of claiming a first-timers-only issue. We're happy to keep it assigned to you as long as you need if you update us, but if we don't see any activity a week after you claim it we may reassign it to give someone else a chance. Thank you in advance!
If this happens to you, don't sweat it! Grab another open issue.
💬 Get help
If you need any help - here are some options:
The text was updated successfully, but these errors were encountered: