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

unique identifier for danger-written comments? #269

Closed
kwonoj opened this issue Jun 6, 2017 · 5 comments · Fixed by #270
Closed

unique identifier for danger-written comments? #269

kwonoj opened this issue Jun 6, 2017 · 5 comments · Fixed by #270

Comments

@kwonoj
Copy link
Member

kwonoj commented Jun 6, 2017

Due to private repo's limitation, I am currently using myself's github token to run danger instance (which post comments under my user). Which makes if I leave a comment before danger finishes run, danger will remove those comment and replace it into danger's results.

Is there way to make danger validate comments (like having some prefix in comments?) to avoid these cases?

@orta
Copy link
Member

orta commented Jun 6, 2017

There is always the "posted by danger" - could verify that that exists inside the comment before declaring it as one posted by the tool?

@orta
Copy link
Member

orta commented Jun 6, 2017

Also HTML stays inside the text comment (even if it's not rendered by markdown) so adding a useful <!-- thing --> is also an option.

@kwonoj
Copy link
Member Author

kwonoj commented Jun 6, 2017

There is always the "posted by danger" - could verify that that exists inside the comment before declaring it as one posted by the tool?

I guess this can be used, question is more about locating comment ID logics (

const dangerComment = find(allComments, (comment: any) => comment.user.id === userID)
) which currently just compares userID directly, if there's configurable way (or even fixed way by validating those).

@orta
Copy link
Member

orta commented Jun 6, 2017

Perhaps then I think a mix of same user and a specific hidden marker which can be added into every comment from danger is optimal.

@kwonoj
Copy link
Member Author

kwonoj commented Jun 6, 2017

@orta I'd 👍 with those idea.

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 a pull request may close this issue.

2 participants