Skip to content

Conversation

@Erroler
Copy link

@Erroler Erroler commented Nov 13, 2018

Fixes Issue #2563

@ZeroX-DG ZeroX-DG added the awaiting review ❇️ Pull request is awaiting a review. label Nov 14, 2018
@Erroler
Copy link
Author

Erroler commented Dec 6, 2018

Is there something else I need to do before the review?

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Dec 6, 2018

No, it's just I'm a little bit busy. I'll try to review this as soon as possible.

@ZeroX-DG
Copy link
Member

ZeroX-DG commented Jan 19, 2019

Can you fix the conflict? Thank you.

@guardrails
Copy link

guardrails bot commented Jan 19, 2019

⚠️ We detected security issues in this pull request:

Insecure Use of Regular Expressions (1)

More info on how to fix Insecure Use of Regular Expressions in Javascript.


Insecure Use of Language/Framework API (1)

More info on how to fix Insecure Use of Language/Framework API in Javascript.

Happy with the results? Give your feedback.

@Erroler
Copy link
Author

Erroler commented Jan 19, 2019

I resolved the conflicts but I really don't understand what's going on with guardrails. Could you please advice me on what to do? The line it is referring to aren't a part of my pull request.

@ZeroX-DG
Copy link
Member

Please ignore it, the bot is crazy:smile:

Copy link
Member

@ZeroX-DG ZeroX-DG left a comment

Choose a reason for hiding this comment

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

I'm very sorry for the long wait 😢 Can you fix your code please?


// "Remember" top line number (property of grand-parent component).
const currentLineAtTop = this.editor.lineAtHeight(this.editor.getScrollInfo().top, 'local') + 1
this.props.topLine.number = currentLineAtTop
Copy link
Member

Choose a reason for hiding this comment

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

Modifying props is consider as anti-pattern for React. Can you do it some otherway? maybe notify parent to update instead of directly modifying like this

@ZeroX-DG ZeroX-DG added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels May 11, 2020
@ZeroX-DG
Copy link
Member

ZeroX-DG commented Jun 2, 2020

@Erroler can you resolve the conflict and change your code please? If you not interested in this PR anymore, I'll take over this after a week

@Erroler
Copy link
Author

Erroler commented Jun 2, 2020

@Erroler can you resolve the conflict and change your code please? If you not interested in this PR anymore, I'll take over this after a week

Feel free to take over.

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

Labels

awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants