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

[WIP]: Update contributing #1096

Closed
wants to merge 12 commits into from
Closed

[WIP]: Update contributing #1096

wants to merge 12 commits into from

Conversation

joshbruce
Copy link
Member

Marked version: 0.3.17

Description

Moving this comment from @intcreator here to continue the broader conversation on helping new contributors get up to speed with marked.

Contributor

  • Test(s) exist to ensure functionality and minimize regresstion (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.
  • cm_autolinks is the only failing test (remove once CI is in place and all tests pass).
  • All lint checks pass (remove once CI is in place).
  • CI is green (no forced merge required).
  • Merge PR

@joshbruce joshbruce added good first issue Something easy to get started with proposal labels Feb 28, 2018
@joshbruce joshbruce mentioned this pull request Feb 28, 2018
@joshbruce
Copy link
Member Author

@intcreator: If you would like to talk privately, we can find a different venue, but you're one of the first to ask regarding new contributor on-boarding; so, wanted to acknowledge that and get to know the issues you're having a bit more. We can also try something other than text-based communication, if you would prefer.

@intcreator
Copy link

That looks like a good start.

Browser compatibility: We should probably support modern browsers (Chrome, Firefox, Edge, and Safari current and previous two versions) and (sigh) IE 11. As far as I know twice as many people still use IE as Edge. I think that set of browsers is pretty standard. Vue and React support that same set of browsers except they go back to IE 9. If anyone really wants to, we can make it happen, but IE 11 is probably painful far enough.

Node support: I know less about what versions of Node are currently in widespread use. Perhaps we can support the oldest version supported by Node.js itself? Node 4 will be in maintenance until April so I wouldn't mind passing it by. Node 6 will enter maintenance in April and continue until April 2019 so maybe we can start there.

@joshbruce
Copy link
Member Author

@intcreator: Thanks!

I updated the Submitting PRs continued section to account for where we are right now. Let me know your reaction to that.

Also added the The marked architecture section. For that one, if you had to guess, how would you describe each of the parts that have the ellipses (...)?

Tagging @styfle as well for situational awareness and silent partner for now.

@UziTech
Copy link
Member

UziTech commented Feb 28, 2018

Right now I'm pretty sure marked works all the way back to IE8 (some people just don't have enough XP) and our travis-ci runs node 0.10. I think those are good limits for 0.x

Node 6 is the first to support 99% of ES6. I agree with following the lifecycle of node.

I would say that starting with 2.x we only support the latest evergreen browsers (i.e. not IE) and follow node lifecycles (so probably Node 6)

@styfle
Copy link
Member

styfle commented Feb 28, 2018

I would say that starting with 2.x we only support the latest evergreen browsers (

Which browsers should be supported for 1.x then?

@UziTech
Copy link
Member

UziTech commented Feb 28, 2018

same as 0.x

@joshbruce
Copy link
Member Author

joshbruce commented Feb 28, 2018

This is interesting to me from the perspective of the CLI (not a browser)...we're not really dependent on the browsers per se, yeah? Instead we are dependent on the JS engine running marked (browsers just happen to come with an engine), just a thought (could totally be wrong, if I am let me know).

@UziTech
Copy link
Member

UziTech commented Feb 28, 2018

You are correct. The browser/node version just comes with a certain JS engine that runs marked.

That is why browsers started auto updating so we can just say we support the latest stable version of each browser.

Wouldn't it be awesome if node started auto updating 🤣

@intcreator
Copy link

intcreator commented Feb 28, 2018

I think it would also be a good idea to mention which Markdown specifications Marked will adhere to and whether or not Marked can add features with flags or maintain itself as an easily extensible base.

The README mentions that both CommonMark and GFM are supported, but I would go into a little more detail on the mechanics. For example, these questions come to mind:

  1. Do we always start to adhere to the latest version of the spec immediately or wait to adopt it at a certain version/milestone?
  2. What happens when a user uses CommonMark vs GFM under the hood?
  3. Do we have plans for supporting other flavors/dialects of Markdown?

Edit: I just discovered USAGE_EXTENSIBILITY.md, which may be a better home for some of this information.

@joshbruce
Copy link
Member Author

joshbruce commented Feb 28, 2018

@UziTech: Funny story. I'm on a project where the platform automatically updated Node for security - broke all our stuff...so, not sure I can come with you on the autoupdate; unless we're strictly speaking about the JS engine. 😝

@intcreator: Let's put a pin in that. Gonna create an issue.

@joshbruce joshbruce mentioned this pull request Feb 28, 2018
@joshbruce
Copy link
Member Author

@intcreator: Just to make sure this was seen and is covered (we sometimes get sidetracked and lose a request in the excitement of the conversation and collaboration).

Also added the The marked architecture section. For that one, if you had to guess, how would you describe each of the parts that have the ellipses (...)?

@styfle
Copy link
Member

styfle commented Feb 28, 2018

same as 0.x

@UziTech If we're not dropping support for anything in 1.0.0 then what is the breaking change? And what is the timeline to release 1.0.0?

@UziTech
Copy link
Member

UziTech commented Feb 28, 2018

what is the breaking change?

Most likely a bunch of stuff

A lot of projects that I have seen depend on marked use a specific version (i.e. "0.3.17") because something could easily break while everything is changing (spec support, etc.)

The idea behind v1.x is that dependents can finally use "^1.0.0" and not have to worry about us deciding that we are or aren't going to support a spec.

@intcreator
Copy link

intcreator commented Feb 28, 2018

The first two FAQs in Semantic Versioning suggest to me that 1.0 doesn't require breaking change but rather signifies a stable release/API.

Nevertheless, I'm sure if we get Marked where we want by 1.0, it will break something. It's software after all...

@joshbruce I'm kind of at a dead end on architecture. I really don't understand what's going on, so I think it would be invaluable to have a previous contributor or two write and inspect that section for accuracy.

@joshbruce
Copy link
Member Author

joshbruce commented Feb 28, 2018

@intcreator: Fair. Thanks for everything so far. Keep going!

@UziTech and @styfle: Anyone we can ask (or beg)? Maybe @Nalinc from #746

Just to make it easier to see the ask:

What is the single responsibility or purpose of the things identified with ellipses (...)??

@joshbruce
Copy link
Member Author

joshbruce commented Feb 28, 2018

All right @intcreator, @UziTech, @styfle, and @davisjam - My guesses on what the single responsibility of the objects do is in...am I wrong, do you think. See also #1102

Gonna be done for the day. Need to catch up on some things. Let me know if something explodes. 😂

3. `edit()`: ...
4. `resolveUrl()`: ...
5. `noop()`: ...
6. `merge():` Merges one or more objects to create a new object. (Why not use Object.assign??)

Choose a reason for hiding this comment

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

Object.assign is an ES6 feature according to MDN, so it'll have to wait until 2.0

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

This is failing with merge conflicts since we moved our docs.


However, marked has also been around for several years and is still one of the most popular Markdown parsers we're aware of. Therefore, we can't ethically just change everything out from under our users before making things right.

### Release 0.x.x (no known issues)
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that CONTRIBUTING is the place to talk about releases.
That could be in a wiki roadmap...see TS Roadmap

Additionally, the description of the milestone should actually live on the milestones

@joshbruce
Copy link
Member Author

@styfle: Gonna close and make a note to update it later. Thinking Roadmap will be part of the docs?? Trying to minimize the places a developer has to go to learn about all the things.

@joshbruce joshbruce closed this Mar 26, 2018
@joshbruce joshbruce deleted the update-contributing branch March 26, 2018 03:05
@joshbruce
Copy link
Member Author

See #1162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Something easy to get started with proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants