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

fix: change organization of project repository to mdx-js :tada: #109

Merged
merged 1 commit into from Dec 10, 2019
Merged

Conversation

@JounQin
Copy link
Member

JounQin commented Dec 1, 2019

No description provided.

@codecov-io

This comment was marked as resolved.

Copy link

codecov-io commented Dec 1, 2019

Codecov Report

Merging #109 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #109   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          19     19           
  Lines         402    402           
  Branches       75     75           
=====================================
  Hits          402    402

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b397367...275b9dc. Read the comment docs.

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 1, 2019

@wooorm Hi, I'm blocked by codecov from merging this PR, would you like to help with this about accepting permission requests of codecov?

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 1, 2019

@ChristianMurphy Thx, but it's not about PR approve, I enabled codecov check before migrating it into mdx-js organization, so can you enable codecov for this repository?

image

@ChristianMurphy

This comment has been minimized.

Copy link
Member

ChristianMurphy commented Dec 1, 2019

@JounQin for now I've made codecov optional to unblock this PR. ✔️
I'll defer to @wooorm and @johno on enabling codecov.

Personally I prefer using https://jestjs.io/docs/en/configuration#coveragethreshold-object to pass/fail builds, over getting codecov comments on every commit, but that's just me 🤷‍♂

@wooorm

This comment has been minimized.

Copy link
Member

wooorm commented Dec 1, 2019

Agreed with Christian. I’m OK with enabling codecov. Will wait for John to enable it, or do it myself in a couple of days!

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 1, 2019

Actually I've enabled thread already.
https://github.com/mdx-js/eslint-mdx/blob/master/package.json#L63-L74
All 100% coverage 😎

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 1, 2019

It seems I'm still 'not authorized to merge this pull request', please help to check
my repository permissions, thx guys.

I've added master branch as protected before.

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 1, 2019

@wooorm
'The base branch restricts merging to authorized users.'

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 10, 2019

@johno Would you like to help reviewing my permissions for merging PRs of this repository?

@wooorm

This comment has been minimized.

Copy link
Member

wooorm commented Dec 10, 2019

@JounQin I have removed the branch protection rules that were in this repository!

@wooorm wooorm merged commit 5f38998 into master Dec 10, 2019
2 checks passed
2 checks passed
Travis CI - Branch Build Passed
Details
Travis CI - Pull Request Build Passed
Details
@wooorm wooorm deleted the chore/org branch Dec 10, 2019
@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 10, 2019

@wooorm Then another question here is that it seems I've lost npm publish permission of eslint-mdx and eslint-plugin-mdx. I used to publish npm packages with lerna automatically. Any advice here? Or can you change the npm token in settings of Travis CI?

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 11, 2019

@mdx-js/core @mdx-js/maintainers

OK, I found out that I'm not a npm releaser right now. So would you like to help publishing a new version 1.6.4 of these two packages manually and change the NPM_TOKEN environment in settings of Travis CI so that the workflow won't be broken in the future?

@ChristianMurphy

This comment has been minimized.

Copy link
Member

ChristianMurphy commented Dec 11, 2019

@mdx-js/releasers ☝️

@wooorm

This comment has been minimized.

Copy link
Member

wooorm commented Dec 11, 2019

I'll leave publishing to releasers.

Regarding the token: I'm not sure about using an npm token in Travis to automatically release projects, as it essentially let's anyone with GitHub write access now publish to npm. That's a security problem.
This is a general opinion though, if a project moves fast and isn't used by a lot of people yet, it may be fine 🤷‍♂️

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 11, 2019

@wooorm That's fine to me, but another question is: is there any releaser interested into this repository and gonna to release them on a schedule.

Besides, another opinion here is that we've guarded codes been merged into master branch by mergers, and we can enable only releasing on master branch, then why a security problem will exist?

@wooorm

This comment has been minimized.

Copy link
Member

wooorm commented Dec 11, 2019

I'm sure someone will cut a release soon.

It appears you'd like for releases to happen multiple times a day or every day. That may make sense for this project, but it doesn't make for a very stable ecosystem. MDX is run by volunteers who aren't around every day. Unified is 100s of projects that move at a slower pace because it's been around for years and will be around for years to come. Similar to how React isn't cutting a new release every day either. (Note that the mdx team is free to decide how to do things like this, themselves, it's not a requirement from unified)

Regarding securing master: that was the previous behaviour and you weren't allowed to merge on master. It's the same thing, but making every merger into a releaser.
So the mdx team can change that and secure branches and therefore make every merger a releaser. That is their choice.

@JounQin

This comment has been minimized.

Copy link
Member Author

JounQin commented Dec 11, 2019

@wooorm This repository is stable enough for now, so I'm very open to disable releasing automatically on CI, but also I'm waiting for more responses from other releasers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.