Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
GRS Action 🎉🎉 #2537
base: master
Are you sure you want to change the base?
GRS Action 🎉🎉 #2537
Changes from all commits
26a8a23
33ad0f4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed on discord, this can be removed and enabled by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will implement that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool thanks! People can still disable it by changing the environment variable. This variable has to be documented in the GRS readme. We can do that later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you remove the full markdown code? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When using the GitHub action, you don't give options through the URL. Instead you give the ?param=something&... options as an input for the action. Having the markdown code will make it confusing for those using the action. Instead, I've included instructions at the top on how to customize the card for the action version and the hosted version, and I'm just mentioning the custom options below. This also makes the documentation cleaner in my opinion. No need to keep repeating the markdown code if it's already specified at the top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already suspected that. I prefer having the full card generation code in GRS and a note in the GitHub action about this. I am afraid many beginners or lazy people will create new issues and discussions. We can document the difference in the action README. If you want, you can even add a warning/Note to make it more clear:
Anyway, if @anuraghazra thinks your change is fine, I'm also okay with it. 👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is temporary right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the file path is an input, the filename can be whatever the user chooses. Moreover, we directly do not push to any branch. Depending on which action they use to push, the raw url of the svg may be different. Finally, GitHub raw usercontent is blocked by Jio, which is one of the largest ISPs in India. This means that many, me included, can't view GitHub raw files. Initially I built my own proxy, but now I just use GitHub Pages. Either one will change the url. I've included instructions on how to find this URL above, and they just have to replace SVG_URL in the sample code with that url.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think I agree with you on this point. If we clearly describe the
<DARK_SVG_URL>
and<SVG_URL>
above it should be fine.BTW if you would love to make the documentation more clear, I think you will love the PR I and @mezzode did in #2242.