-
-
Notifications
You must be signed in to change notification settings - Fork 9
Provide a pre-built SourceCode cog #310
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
Conversation
2058c10 to
a7817f0
Compare
57016b4 to
1195653
Compare
| if not source_item: | ||
| embed = Embed(title=f"{self.bot.user.name}'s GitHub Repository") | ||
| embed.add_field(name="Repository", value=f"[Go to GitHub]({self.github_repo})") | ||
| embed.set_thumbnail(url=GITHUB_AVATAR) | ||
| await ctx.send(embed=embed) | ||
| return |
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.
thoughts on adding a link to bot core as well here?
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.
Noise for most use cases, pydis_core is never really relevant to those outside PyDis (hence we don't really widely promote it).
Supporting commands from pydis_core is about the most we should support I think.
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.
Most of time someone looking at the source command is trying to contribute and looking at how commands are implemented. Giving a link to where other commands are implemented is a benefit and intent of the source command, IMO.
If a user is using the bot's source command, they're already interested in how it works, and surfacing bot-core is an important part of how the pydis bots work.
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 don't see a nice way to include this without fundamentally complicating what is a simple feature that hundreds of bots have.
If someone else feels passionately that we should include it, then sure, but the whole point here is to try and transparently carry across functionality from the current command, which obviously does not mention pydis_core.
|
|
||
| if source_type == _SourceType.core_command or source_type == _SourceType.core_cog: | ||
| version = f"v{metadata.version('pydis_core')}" | ||
| elif sha := os.getenv("GITHUB_SHA"): |
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.
this could be provided as a setup variable with a default of "main"
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.
It could, but this works with our current bots without change on their side. Just seems easier and is guaranteed to work for our use cases.
Setup for the bot core instance is already massive.
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.
Fair enough, I think. Unified configuration should help this some.
c1a1841 to
d2bb444
Compare
6cbc7f9 to
3b5c355
Compare
3b5c355 to
2fa5aa2
Compare
Copies over the source command from bot, including functionality that allows for tag parsing on bot.
Closes #306