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 PLD Linux package link to project page #1065

Merged
merged 1 commit into from
Mar 18, 2021

Conversation

glensc
Copy link
Contributor

@glensc glensc commented Mar 17, 2021

Similarly to #1053

@glensc glensc requested a review from a team as a code owner March 17, 2021 11:41
Copy link
Contributor

@Zlopez Zlopez left a comment

Choose a reason for hiding this comment

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

Could you also add a news file. See https://anitya.readthedocs.io/en/latest/contributing.html#release-notes or look at the #1053

@glensc
Copy link
Contributor Author

glensc commented Mar 17, 2021

Could you also add a news file. See anitya.readthedocs.io/en/latest/contributing.html#release-notes or look at the #1053

Added. However, I think the change is rather trivial not worth the changelog entry:

Also, I think the effort for changelog creation, is too much for developers. Especially, if want to make a simple change from GitHub Web. Traversing files, finding where to add them, and whats the syntax. Bleh. You should look into better release workflows that can obtain the changelog/release notes from pull request titles and labels.

I personally have been keeping an eye on https://github.com/laminas/automatic-releases

My 2¢ (not attached)

@codecov
Copy link

codecov bot commented Mar 17, 2021

Codecov Report

Merging #1065 (52f0070) into master (215adea) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1065   +/-   ##
=======================================
  Coverage   97.38%   97.38%           
=======================================
  Files          65       65           
  Lines        4285     4285           
  Branches      598      598           
=======================================
  Hits         4173     4173           
  Misses         65       65           
  Partials       47       47           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 215adea...52f0070. Read the comment docs.

@Zlopez
Copy link
Contributor

Zlopez commented Mar 17, 2021

I only want the news file for changes that would be interesting for users of the application. If this is just some small change in documentation or fix for some typo in code I wouldn't care about it.

@@ -0,0 +1 @@
Add PLD Linux package link to project page. #1065, #1064
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the link from the news file, those aren't needed.

Suggested change
Add PLD Linux package link to project page. #1065, #1064
Add PLD Linux package link to project page.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How else would you have a link to both of them in changelog?

Copy link
Contributor

@Zlopez Zlopez Mar 17, 2021

Choose a reason for hiding this comment

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

No need to link both of them, just the issue itself, or PR if there is no issue. I see that you are using #1065 instead of #1064 in the name. This wouldn't work because the link will point to issue with id 1065, which doesn't exists.

I would recommend to rename the news file to 1064.feature

Copy link
Contributor Author

@glensc glensc Mar 17, 2021

Choose a reason for hiding this comment

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

This wouldn't work because the link will point to issue with id 1065, which doesn't exists.

yes, it will work tecnically, GitHub has a unique id namespace for issues and pull requests, and they both redirect to each other:

  • https://github.com/fedora-infra/anitya/issues/1065
  • https://github.com/fedora-infra/anitya/pull/1064

I would rather rename it to PR1065.feature then, as while it #1065 covers #1064, it doesn't solve it, i.e it's a rather open question for documentation or something.

however, I can't do file rename from GitHub web, so need to check out this locally to make changes, so this will take time.

(edited example links, as GitHub rendered the links as #NUMBER, making the point not understandable)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for doing this

@Zlopez Zlopez linked an issue Mar 17, 2021 that may be closed by this pull request
@Zlopez
Copy link
Contributor

Zlopez commented Mar 17, 2021

I was able to test it on the local instance of Anitya and the link is working fine, it redirected me to the source repo of the project.

Copy link
Contributor

@Zlopez Zlopez left a comment

Choose a reason for hiding this comment

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

Change name of the news file to point to correct issue.

@Zlopez Zlopez removed a link to an issue Mar 17, 2021
@Zlopez Zlopez merged commit 9d4201e into fedora-infra:master Mar 18, 2021
@glensc glensc deleted the patch-1 branch March 18, 2021 11:40
@glensc
Copy link
Contributor Author

glensc commented Mar 18, 2021

Thanks, @Zlopez this will help to verify matched package validity more easily!

@Zlopez
Copy link
Contributor

Zlopez commented Mar 18, 2021

No problem, I will merge the current pull requests and release a new version, so it will be in release-monitoring.org soon.

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