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

Delete duplicate beforeValueRender jsdoc #4012

Merged
merged 1 commit into from Jun 1, 2017
Merged

Delete duplicate beforeValueRender jsdoc #4012

merged 1 commit into from Jun 1, 2017

Conversation

corburn
Copy link

@corburn corburn commented Jan 20, 2017

Context

The beforeValueRender event is documented twice, this commit removes one.

pluginHooks.js#L702

pluginHooks.js#L1122

How has this been tested?

untested

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Related issue(s):

Checklist:

  • My code follows the code style of this project,
  • My change requires a change to the documentation.

@corburn corburn changed the base branch from master to develop January 26, 2017 22:14
@AMBudnik
Copy link
Contributor

AMBudnik commented May 19, 2017

Hi

Thank you very much for sharing the pull request with us!

Yesterday we have changed the Contributing policy, a new file can be found here. As you can see the first obligatory step is to sign a CLA document that will allow us to check your code and (if it meets project requirements) merge it to Handsontable.

As most of the PR in our repository are 3 months+ old we have decided to close them all and reopen immediately after the author confirms that all the steps of the Contributing file have been accomplished.
In the same time, I am really sorry that we have waited so long to provide the correct pull request process. I hope that now we will be able to build the perfect library together :)

ps. if you have added more than one pull request please share all their numbers in one post. Thanks in advance!

@AMBudnik AMBudnik closed this May 19, 2017
@corburn
Copy link
Author

corburn commented May 19, 2017

All the steps in the Contributing file have been accomplished.

@AMBudnik AMBudnik reopened this May 22, 2017
@AMBudnik
Copy link
Contributor

Thank you, your PR is ready for a review. It will be reviewed as soon as possible.

@krzysztofspilka
Copy link
Member

@AMBudnik as a rule of thumb, we should schedule reviews more precisely. In this case, for instance, we can look at this on Friday (or next Friday at the latest). What do you think about such approach?

PS it requires some preparation during a week and a short notice/summary to be sent on Friday to the dev team.

@AMBudnik
Copy link
Contributor

@krzysztofspilka sure! If something isn't scheduled it can hang on the to-do list forever. We can set the 'review' time from one Friday to another, which gives us a full week starting from the point of picking up a PR to actually returning some feedback.

What we should consider though is the fact that one PR can change 2 lines of code like correcting typos in documentation and another can add 10k lines of code and a new feature.

@budnix budnix merged commit b20516a into handsontable:develop Jun 1, 2017
@budnix
Copy link
Member

budnix commented Jun 1, 2017

Thanks, @corburn for PR!

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.

None yet

5 participants