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

metadata: add repository #45

Merged
merged 1 commit into from
Jun 14, 2019
Merged

Conversation

mathstuf
Copy link
Contributor

@mathstuf mathstuf commented Sep 25, 2018

Cargo guidelines are to not set a homepage if it isn't any different
than the repository URL. This causes the report to not have any link
back to file an issue. If the repository is available, print it as well.


Note that I've tried to find a reference to the "don't set homepage if it isn't the same" guideline, but haven't found it yet.

The environment variable is not available yet. See rust-lang/cargo#6096

Choose one: is this a 🙋 feature?

Checklist

  • tests pass
  • documentation is changed or added

Context

None

Semver Changes

Major; the Metadata structure has a new field that isn't specified in existing code.

Copy link
Collaborator

@spacekookie spacekookie left a comment

Choose a reason for hiding this comment

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

I'm not sure that such a small change is enough to warrant pushing a 2.0 😏

We might want to do a 2.0 at some point but maybe combine that change with a few more substantial design changes.

Until then we could think about adjusting the docs to make people aware of this, i.e. making sure developers are aware that the homepage should be a good place to both find ways to report issues as well as general information about the app they're using.

@mathstuf
Copy link
Contributor Author

Understood, but how about this instead:

  • Open an issue to do this for 2.0 (to keep a tracker on it);
  • Use CARGO_PKG_REPOSITORY for Homepage: if CARGO_PKG_HOMEPAGE is empty (possibly with a note about it being the repository).

@mathstuf
Copy link
Contributor Author

making sure developers are aware that the homepage should be a good place to both find ways to report issues as well as general information about the app they're using.

That's fine if I have one, but all I have is a repository (hence this change) :) .

@spacekookie
Copy link
Collaborator

Yea but at that point you have the option of making a Github landing page or making the README friendly enough to read for non-developers. At least that's how I've been handling it for now.

I'm not opposed to a 2.0 tracking issue where this PR might eventually land, although I doubt there's gonna be enough new rationale to warrant ever doing a 2.0 (my co-maintainers might disagree with me here 😉)

Copy link
Collaborator

@spacekookie spacekookie left a comment

Choose a reason for hiding this comment

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

Hey @mathstuf, I'm currently doing some prep for a v2.0.0 release and I'd like to merge this code.

Could you have a look at re-formatting the code according to what the CI wants?

Cargo guidelines are to not set a homepage if it isn't any different
than the repository URL. This causes the report to not have any link
back to file an issue. If the repository is available, print it as well.

Signed-off-by: Ben Boeckel <ben.boeckel@kitware.com>
@mathstuf
Copy link
Contributor Author

Done. Looks like the CI needs to bump to testing 1.31 as the minimum version is effectively that based on dependencies :/ .

@spacekookie spacekookie merged commit 62c4e0a into rust-cli:master Jun 14, 2019
@mathstuf mathstuf deleted the repository-info branch June 14, 2019 14:19
spacekookie added a commit that referenced this pull request Mar 22, 2020
This reverts commit 62c4e0a, reversing
changes made to f4a5b23.
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