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

Add a description of the scopeQuery method #916

Merged
merged 3 commits into from
Apr 15, 2019

Conversation

hodbadger
Copy link

@hodbadger hodbadger commented Mar 22, 2019

Fixes #885

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

  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with npm test
  • code is in uniquely-named feature branch and has no merge conflicts
  • PR is descriptively titled
  • ask @publiclab/is-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 Mar 22, 2019

Thanks for opening this pull request!
There may be some errors, but don't worry! We're here to help! 👍🎉😄

Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

Nicely done but requires a few changes.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
@harshkhandeparkar
Copy link
Member

Please link to the issue in the pr body like I just did by editing your pr.

Also please have a look at #833

@hodbadger
Copy link
Author

Sorry to bother you but I have some questions.
I have to create a new pull request with the required changes, right?
Also, I've already taken a look at #833, but what exactly should I pay attention to?
I'm also not sure what you mean by linking the issue in the pr body - where should I put the link?

@harshkhandeparkar
Copy link
Member

harshkhandeparkar commented Mar 22, 2019

Ok, answers:

  1. You don't have to open a new pr, you can commit to the same branch and the commits will be visible here.
  2. Completion Of $step helper method #833 mentions that all 3 PRs will be merged together. So, this pr will not be merged instantly, you will have to wait for the 3rd pr. You can solve other issues, no problem. There are some other documentation issues but they are not first timers only issues but you are no longer a first timer.
  3. The pr template has "Fixes #[add issue number here.]". This can be replaced with for e.g. Fixes #220 if the issue you are solving is the 220th issue. When the pr is merged issue number 220 will be automatically closed. This also holds true if you use keywords like fix, resolve, resolves, closes etc. The issue will be closed upon merge. So, next time you can add the issue number to the pr (if your pr fixes an issue). This time I added the issue number to demonstrate. Github automatically changes #xxx where xxx is a number to a link to issue number xxx. You can also add the full browser link to the issue and github will also change that to #xxx as a link.

@hodbadger
Copy link
Author

Thanks a lot for the answers!
I've committed the changes and hope it's OK now.
I'll be glad to help with other issues connected to documentation.

Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

Great! But a few more changes are required and the code has a small syntax error.

CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
Copy link
Member

@harshkhandeparkar harshkhandeparkar left a comment

Choose a reason for hiding this comment

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

Great!! Thanks for your efforts and welcome to the publiclab coding community

@hodbadger
Copy link
Author

Thanks for your patience and for helping me through the issue. As I said, I'd be glad to help with other documentation issues (the only thing I can help with since I'm not a developer).

@harshkhandeparkar
Copy link
Member

You can check other issues with the documentation label. Thanks!

@jywarren jywarren merged commit 6fa8b1b into publiclab:main Apr 15, 2019
@welcome
Copy link

welcome bot commented Apr 15, 2019

Congrats on merging your first pull request! 🙌🎉⚡️
Your code will be published to https://beta.sequencer.publiclab.org in a day or two.
In the meantime, can you tell us your Twitter handle so we can thank you properly?
Now that you've completed this, you can help someone else take their first step!
See: Public Lab's coding community!

@jywarren
Copy link
Member

Thank you so much!!!

@harshkhandeparkar
Copy link
Member

@jywarren the scopeQuery method which is documented is non existent 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Document scopeQuery helper method
3 participants