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

Updating to use Treeware #156

Merged
merged 2 commits into from
Mar 11, 2020
Merged

Conversation

jamesmills
Copy link
Contributor

Adding Treeware "conditions of use" test. Also adding basic badge. As requested by Mr Phil Sturgeon.

Adding Treeware "conditions of use" test. Also adding basic badge. As requested by Mr Phil Sturgeon.
@coveralls
Copy link

coveralls commented Mar 10, 2020

Pull Request Test Coverage Report for Build 395

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 93.908%

Totals Coverage Status
Change from base Build 393: 0.0%
Covered Lines: 679
Relevant Lines: 712

💛 - Coveralls

Copy link
Member

@JamesMessinger JamesMessinger left a comment

Choose a reason for hiding this comment

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

Thanks for adding the badge and text! I love the "treeware" idea and look forward to helping it catch-on in the JavaScript community like it has started to in the Laravel community.

I've suggested a couple changes. Let me know what you think. The important one is the link, which currently gets a 404 error.

README.md Outdated Show resolved Hide resolved
@@ -8,6 +8,7 @@ JSON Schema $Ref Parser
[![npm](https://img.shields.io/npm/v/json-schema-ref-parser.svg)](https://www.npmjs.com/package/json-schema-ref-parser)
[![Dependencies](https://david-dm.org/APIDevTools/json-schema-ref-parser.svg)](https://david-dm.org/APIDevTools/json-schema-ref-parser)
[![License](https://img.shields.io/npm/l/json-schema-ref-parser.svg)](LICENSE)
[![Buy us a tree](https://img.shields.io/badge/Treeware-%F0%9F%8C%B3-lightgreen)](https://click.treeware.earth/APIDevTools/json-schema-ref-parser)
Copy link
Member

Choose a reason for hiding this comment

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

This link goes to a 404 page. Is that just a temporary problem? Should we instead link directly to https://offset.earth/treeware?gift-trees instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's best you leave the link to go via our click URL. This will allow us to track the trees in our forest which came from your project. This is a work in progress as I work with Offset.Earth guys on this.

The URL will work once you have added the Treeware licence and then added your package to the website https://treeware.earth/#add-package-section (our website checks you have the licence in your README).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Guys, I have added a quick message on the website to show you that the Click URL will work once you have added the README and added the package to the Treeware website.

https://click.treeware.earth/APIDevTools/json-schema-ref-parser

Hope this helps.

Copy link
Member

Choose a reason for hiding this comment

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

That looks great @jamesmills. Much better and more informative than the empty 404 page 👍

Question though... what exactly do you check for to verify that the Treeware license has been added to the ReadMe? Does it require certain wording? Or does it just check for the badge? Or just the link?

Why even require a check at all? Seems like an unnecessary roadblock to me that will lower the adoption rate of Treeware. Maybe somebody doesn't want to add anything to their ReadMe, but they want to link to Treeware in their docs, or maybe even in their product.

@JamesMessinger JamesMessinger merged commit 79a0dd2 into APIDevTools:master Mar 11, 2020
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 this pull request may close these issues.

None yet

4 participants