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 RepositoryAuthor and RepositoryName in BuildInfo #300

Closed
wants to merge 1 commit into from

Conversation

mctaylors
Copy link
Member

#300 🥳

Anyway, this PR add two new const strings named RepositoryAuthor and RepositoryName. This was made for future PR (which will be open in about 5-10 minutes).

@mctaylors mctaylors added the type: change PRs that update current functionality without adding new features or fixing any bugs label Apr 7, 2024
@mctaylors mctaylors requested a review from a team as a code owner April 7, 2024 12:52
@pull-request-size pull-request-size bot added the size/XS PRs that change <20 lines. label Apr 7, 2024
@Octol1ttle
Copy link
Member

If you need this just for #301, I would reject this because this solves very little use cases. What if the image isn't hosted on GitHub? What if the path to the image is different?
At this point, just put the whole link in a const, without any interpolation

@mctaylors
Copy link
Member Author

What if the path to the image is different?

...then what's stopping a developer from changing it as he pleases?

At this point, just put the whole link in a const, without any interpolation

Even if we declare const string for the logo:

  1. we will still need to dynamically change the reference to the logo (for example: if the author/repository name changes)
  2. it will again be a const string for a single use in /about. I just can't think of any other uses for it.

@Octol1ttle
Copy link
Member

What is the point of this pull request? I thought that you added this here for developers to have an easier time changing the logo URL, but your own points seem to disprove that

@mctaylors
Copy link
Member Author

What is the point of this pull request? I thought that you added this here for developers to have an easier time changing the logo URL, but your own points seem to disprove that

Perhaps (if you insist) I will create a separate const string for the logo in BuildInfo, but I think RepositoryAuthor and RepositoryName will still be useful in some places.

@Octol1ttle
Copy link
Member

In which places?

@mctaylors
Copy link
Member Author

In which places?

No exact places yet, and I also have future plans for them.

@Octol1ttle
Copy link
Member

In that case, please implement the changes when these "future plans" come.

@Octol1ttle Octol1ttle closed this Apr 8, 2024
@mctaylors mctaylors added the status: stale This PR is no longer maintained. label Apr 8, 2024
@mctaylors mctaylors deleted the set-repo-author branch June 25, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS PRs that change <20 lines. status: stale This PR is no longer maintained. type: change PRs that update current functionality without adding new features or fixing any bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants