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

Add contribute.json #12251

Merged
merged 2 commits into from
Feb 5, 2020
Merged

Add contribute.json #12251

merged 2 commits into from
Feb 5, 2020

Conversation

jaywink
Copy link
Member

@jaywink jaywink commented Feb 5, 2020

This document is required for example for all Mozilla websites.
Might make sense for Riot sites to also describe themselves?

See: https://www.contributejson.org/

Signed-off-by: Jason Robinson jasonr@matrix.org

This document is required for example for all Mozilla websites
and makes sense for Riot to also describe itself - see
https://www.contributejson.org/

Signed-off-by: Jason Robinson <jasonr@matrix.org>
@jaywink jaywink requested a review from a team February 5, 2020 13:27
@@ -0,0 +1,17 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

According to the docs it should be at the project root?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's copied into webapp in the build here and as such is in the same place as config.json if I've understood the build process correctly?

Copy link
Member

Choose a reason for hiding this comment

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

This is more what I'm referencing:
image

Copy link
Member

Choose a reason for hiding this comment

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

the build tools can pull from there too, though it'll also need adding to the files in the package.json

Copy link
Member Author

Choose a reason for hiding this comment

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

This is more what I'm referencing:
image

Ah right, you're referring to how it's deployed on riot.im/app for example. True, this wont work on the NV maintained riot web's at all. I guess the idea was not so much the NV installed riot but any hosted riot on a root path of their domain.

Copy link
Member Author

Choose a reason for hiding this comment

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

the build tools can pull from there too, though it'll also need adding to the files in the package.json

res is already in files - will that do? I verified yarn build does end up copying the file, but if it's needed explicitly somewhere else can add.

Copy link
Member

Choose a reason for hiding this comment

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

The problem has nothing to do with the build process: I literally mean move this file up one directory.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I seriously have had enough coffee but somehow managed to totally misunderstand everything you clearly wrote while juggling other things. Fixed now.

Copy link
Member

Choose a reason for hiding this comment

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

No worries, thanks!

Signed-off-by: Jason Robinson <jasonr@matrix.org>
@turt2live turt2live self-requested a review February 5, 2020 14:29
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

seems like a thing we should have. If people disagree, they should speak up.

@turt2live turt2live merged commit b107da0 into develop Feb 5, 2020
@t3chguy t3chguy deleted the jaywink/add-contribute-json branch May 12, 2022 09:07
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.

2 participants