Skip to content

Defib challenge badge - #1217

Merged
styfle merged 5 commits into
markedjs:masterfrom
8fold:defib-challenge
Apr 20, 2018
Merged

Defib challenge badge#1217
styfle merged 5 commits into
markedjs:masterfrom
8fold:defib-challenge

Conversation

@joshbruce

Copy link
Copy Markdown
Member

Marked version: 0.3.19

Markdown flavor: CommonMark & GitHub Flavored Markdown

Description

Adds limited badge related to the defibrillator challenge (#1216).

Contributor

  • Test(s) exist to ensure functionality and minimize regression (if no tests added, list tests covering this PR); or,
  • no tests required for this PR.
  • If submitting new feature, it has been documented in the appropriate places.

Committer

In most cases, this should be a different person than the contributor.

  • Draft GitHub release notes have been updated.
  • CI is green (no forced merge required).
  • Merge PR

@joshbrucejoshbruce added the category: docs Documentation changes label Apr 13, 2018
@joshbruce
joshbruce requested review from UziTech and styfleApril 13, 2018 15:13
Comment threaddocs/AUTHORS.md
|Name |GiHub handle |Decision making |Badges of honor (tag for questions) |
|:--------------|:--------------|:----------------------------------------|------------------------------------|
|Jamie Davis |@davisjam |Seeker of Security | |
|Steven |@styfle |Open source, of course and GitHub Guru | |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why did you remove me? Am I not allowed to review PRs?

@joshbrucejoshbruceApr 13, 2018

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Of course. You were in there twice. Once in the admin section and once in the committer section. They're concentric circles. Publishers == admins == committers == contributors == users. That's why I'm not in the committers table either. :)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Jamie and Tony aren't admins though. Make sense?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Trust me, if it were that big of a shift...it would have been a separate PR. ;)

styfle
styfle previously approved these changes Apr 13, 2018
@styfle

Copy link
Copy Markdown
Member

It was misleading because in the PR description there is no mention of removing a duplicate

@joshbruce

Copy link
Copy Markdown
MemberAuthor

@styfle: That's fair. Will note for future. Thought the commit message would have been enough. Thanks for asking definitely see where the confusion came from.

@styfle

Copy link
Copy Markdown
Member

Waiting on @UziTech or @davisjam

UziTech
UziTech previously approved these changes Apr 20, 2018

@UziTechUziTech left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@styfle

Copy link
Copy Markdown
Member

For some reason, Snyk looks stuck.
Maybe we should change this check so that it's not required to merge a PR.

Maybe the Snyk team can help use determine why this check is stuck 😅
@adrukh@aarlaud@karenyavine

@joshbruce
joshbruce dismissed stale reviews from UziTech and styfle via 74ee8c1April 20, 2018 14:16
@styfle

Copy link
Copy Markdown
Member

Ok it looks like that merge fixed the Snyk check.
Needs one more approval.

@styfle
styfle merged commit f95dd44 into markedjs:masterApr 20, 2018
@adrukh

Copy link
Copy Markdown

👋

Closing and reopening the PR (or pushing a new commit) re-triggers the check and gives us a second chance to succeed. We'll take a look why that second chance was needed, thanks for bearing with us!

zhenalexfan pushed a commit to zhenalexfan/MarkdownHan that referenced this pull request Nov 8, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category: docsDocumentation changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@joshbruce@styfle@adrukh@UziTech