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

[Templates] Fix links to http://useiconic.com #44597

Merged
merged 2 commits into from Oct 18, 2022

Conversation

javiercn
Copy link
Member

@javiercn javiercn commented Oct 17, 2022

Updates links to point to up-to-date documentation

Description

Updates the links in the readme.md of one of our template files to point out to the newer documentation site.

Customer Impact

Customers who will read this doc will have valid links to use for getting to the icons.

Regression?

  • Yes
  • No

[If yes, specify the version the behavior has regressed from]

Risk

  • High
  • Medium
  • Low

It is updating a file in the templates that doesn't affect how the project is built or runs, only documentation.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@javiercn javiercn requested a review from a team as a code owner October 17, 2022 15:20
@javiercn javiercn added ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. Servicing-consider Shiproom approval is required for the issue and removed ask-mode This issue / PR is a patch candidate which we will bar-check internally before patching it. labels Oct 17, 2022
@ghost
Copy link

ghost commented Oct 17, 2022

Hi @javiercn. Please make sure you've updated the PR description to use the Shiproom Template. Also, make sure this PR is not marked as a draft and is ready-to-merge.

To learn more about how to prepare a servicing PR click here.

@mkArtakMSFT mkArtakMSFT added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Oct 17, 2022
Copy link
Contributor

@blowdart blowdart left a comment

Choose a reason for hiding this comment

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

As a stopgap measure this works for me

@mkArtakMSFT
Copy link
Member

mkArtakMSFT commented Oct 17, 2022

@blowdart , @javiercn I wonder if for the link we should use the suggestion from iconic/open-iconic#60 (comment) instead?

@Mike-E-angelo Maybe you can use archive.org as a work around for now: https://web.archive.org/web/20220506153325/https://useiconic.com/icons/

But maybe the details page are not up-to-date, but better then nothing.

@blowdart
Copy link
Contributor

I think that's worse, as we're relying on another 3rd party caching a 3rd party. While I generally trust archive.org page copyright owners can get things removed.

The best option would be update the whole thing, icons, readme, license. If we can't get into 7.0 rtm then we have more time and that's the way to go, this is just a tactical option right now.

@danmoseley
Copy link
Member

Note this also exists in dotnet/samples, dotnet/maui-samples, dotnet/blazor-samples, dotnet/aspnetcore.docs. I guess those don't need updating on the GA timeline though.
https://cs.github.com/?q=org%3Adotnet+useiconic

@mkArtakMSFT mkArtakMSFT added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Oct 17, 2022
@mkArtakMSFT mkArtakMSFT changed the title [Templates] Fix links to http://useionic.com [Templates] Fix links to http://useiconic.com Oct 17, 2022
@danroth27
Copy link
Member

danroth27 commented Oct 17, 2022

The original links are to "useiconic.com" not "useionic.com". Ionic is a product that happens to have its own set of icons, but I don't think it's related to Iconic or open iconic, which is what we use. I don't think we should be replacing links to Iconic with links to Ionic icons (ionicons). Maybe just remove the links to useiconic.com for now?

Copy link
Member

@danroth27 danroth27 left a comment

Choose a reason for hiding this comment

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

LGTM

@mkArtakMSFT mkArtakMSFT merged commit 681a0ac into release/7.0 Oct 18, 2022
@mkArtakMSFT mkArtakMSFT deleted the javiercn/fix-template-issue branch October 18, 2022 03:53
@mkArtakMSFT
Copy link
Member

Issue covering remaining work: #44606

@masterparser
Copy link

masterparser commented Oct 21, 2022

I think that the website has changed to: https://www.appstudio.dev/app/OpenIconic.html. This new site features the same number of icons, 223, and same licenses as were found at the previous site (useiconic.com/open).

@ghost
Copy link

ghost commented Oct 21, 2022

Hi @masterparser. It looks like you just commented on a closed PR. The team will most probably miss it. If you'd like to bring something important up to their attention, consider filing a new issue and add enough details to build context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants