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

update the story of fruit picker exercise #1826

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

Aliwahid17
Copy link

@Aliwahid17 Aliwahid17 commented Jul 3, 2022

I updated the story of the fruit picker exercise. I try to make it small understandable, and easy to read.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 3, 2022

Dear Aliwahid17

Thank you for contributing to the JavaScript track on Exercism! 💙
You will see some automated feedback below 🤖. It would be great if you can make sure your PR covers those points. This will save your reviewer some time and your change can be merged quicker.

  • ✍️ If your PR is not related to an existing issue (and is not self-explaining like a typo fix), please make sure the description explains why the change you made is necessary.

  • 🔤 If your PR fixes an easy to identify typo, if would be great if you could check for that typo in the whole repo. For example, if you found Unicdoe, use "replace all" in your editor (or command line magic) to fix it consistently.

Dear Reviewer/Maintainer

  • 📏 Make sure you set the appropriate x:size label for the PR. (This also works after merging, in case you forgot about it.)

  • 🔍 Don't be too nit-picky. If the PR is a clear improvement compared to the status quo, it should be approved as clear signal this is good to be merged even if the minor comments you might have are not addressed by the contributor. Further improvement ideas can be captured in issues (if important enough) and implemented via additional PRs.

  • 🤔 After reviewing the diff in the "Files changed" section, take a moment to think about whether there are changes missing from the diff. Does something need to be adjusted in other places so the code or content stays consistent?

Automated comment created by PR Commenter 🤖.

Copy link
Member

@junedev junedev left a comment

Choose a reason for hiding this comment

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

@Aliwahid17 Great that you want to work on this. Currently there is an unfinished sentence and the task was more about the headers of headers for all the tasks and the text below the tasks 1 and 2. Currently the headers describe the what needs to be done technically, not in term of the story.

If this was a preliminary PR you want to expand on, you can make it a draft PR until everything is ready.

@Aliwahid17
Copy link
Author

ok thanks for your feedback

Copy link
Author

@Aliwahid17 Aliwahid17 left a comment

Choose a reason for hiding this comment

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

i changed some few lines from the fruit picker exercise


## 1. Create a callback to be called when the order is successful

Write a callback function called `onSuccess` to be called when the order is successful. It should invoke the imported `notify` function passing a success message to it.
Write a callback function name `onSuccess` which should invoke the import `notify` function.You have to return a `SUCCESS` message when the order is successful.
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean named / with the name and imported?

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

3 participants