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.

Sign up for GitHub

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

Jump to bottom

feat: Support raw github urls #91

Merged
merged 12 commits into from
Nov 20, 2020
Merged

feat: Support raw github urls #91

merged 12 commits into from
Nov 20, 2020

Conversation

ebebbington
Copy link
Member

@ebebbington ebebbington commented Nov 11, 2020 •
edited by Guergeiro
Loading

Fixes #80

Summary

  • Adds support for raw github urls (only versioned of course)
  • Approval 1
  • Approval 2

@ebebbington ebebbington added the Type: Minor Merging this pull request results in a minor version increment label Nov 11, 2020
@ebebbington
Copy link
Member Author

MMMMMmmmmmmm checks pass first time, that. is. what. i'm. talking. about

crookse
crookse previously approved these changes Nov 11, 2020
View reviewed changes
Copy link
Member

@crookse crookse left a comment

Choose a reason for hiding this comment

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

L
G
T
M

@ebebbington
Copy link
Member Author

thanks for the description edit @crookse 👍

saragee3
saragee3 previously approved these changes Nov 11, 2020
View reviewed changes
Guergeiro
Guergeiro previously approved these changes Nov 11, 2020
View reviewed changes
@ebebbington ebebbington dismissed stale reviews from Guergeiro, saragee3, and crookse via 260cc97 November 11, 2020 11:50
@saragee3
Copy link
Member

what change prompted a re-request?

@ebebbington
Copy link
Member Author

I didn't update the --allow-net permission to include raw.githubusercontent.com and github.com, for both the documentation and tests

So in the help message, it didnt include raw github in the install command (which would make any dmm <command> fail, and also we werent testing the permissions properly - there's an extra test that installs dmm with the very specific permissions then runs dmm update to ensure it works as expected

@ebebbington
Copy link
Member Author

I just need to update the dmm docs on website too before this pr can be merged

@ebebbington
Copy link
Member Author

ok docs are in staging now

@ebebbington
Copy link
Member Author

ok ready for review again

crookse
crookse previously approved these changes Nov 20, 2020
View reviewed changes
Guergeiro
Guergeiro previously approved these changes Nov 20, 2020
View reviewed changes
Copy link
Member

@Guergeiro Guergeiro left a comment

Choose a reason for hiding this comment

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

Everything fine by me 😃

@ebebbington ebebbington dismissed stale reviews from Guergeiro and crookse via d24ec69 November 20, 2020 11:39
@ebebbington
Copy link
Member Author

tests pass, looks good

@ebebbington ebebbington merged commit 2d4a25a into master Nov 20, 2020
@ebebbington ebebbington deleted the 80-support-raw-github branch November 20, 2020 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@crookse crookse crookse left review comments

@saragee3 saragee3 saragee3 left review comments

@Guergeiro Guergeiro Guergeiro left review comments

Assignees
No one assigned
Labels
Type: Minor Merging this pull request results in a minor version increment
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support https://raw.githubusercontent.com
4 participants
@ebebbington @saragee3 @crookse @Guergeiro