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.

Sign up for GitHub

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

Jump to bottom

Add sitemap support #18407

Merged
merged 14 commits into from
Jun 25, 2022
Merged

Add sitemap support #18407

merged 14 commits into from
Jun 25, 2022

Conversation

danog
Copy link
Contributor

@danog danog commented Jan 25, 2022 •
edited
Loading

Fixes #11429.
This is my first time working with the codebase, I realize my approach may have been a a bit non-uniform at times, I'm open to suggestions!

@codecov-commenter
Copy link

codecov-commenter commented Jan 25, 2022 •
edited
Loading

Codecov Report

Merging #18407 (4f2c33b) into main (2ad74a5) will increase coverage by 0.38%.
The diff coverage is 16.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #18407      +/-   ##
==========================================
+ Coverage   45.61%   46.00%   +0.38%     
==========================================
  Files         842      842              
  Lines       93192    93267      +75     
==========================================
+ Hits        42506    42903     +397     
+ Misses      43907    43556     -351     
- Partials     6779     6808      +29
Impacted Files Coverage Δ
routers/web/home.go 16.17% <0.00%> (-20.50%) ⬇️
routers/web/explore/repo.go 61.53% <23.52%> (-7.78%) ⬇️
routers/web/explore/user.go 60.52% <23.52%> (-11.15%) ⬇️
routers/web/web.go 90.55% <100.00%> (+0.05%) ⬆️
models/asymkey/gpg_key_common.go 59.67% <0.00%> (-4.84%) ⬇️
modules/log/file.go 73.60% <0.00%> (-1.61%) ⬇️
routers/api/v1/repo/pull.go 28.35% <0.00%> (-0.51%) ⬇️
modules/queue/queue_bytefifo.go 54.05% <0.00%> (-0.46%) ⬇️
services/pull/pull.go 42.39% <0.00%> (-0.41%) ⬇️
models/repo_list.go 76.09% <0.00%> (+0.58%) ⬆️
... and 5 more

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 2ad74a5...4f2c33b. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 25, 2022
KN4CK3R
KN4CK3R reviewed Jan 25, 2022
View reviewed changes
modules/context/context.go Outdated Show resolved Hide resolved
KN4CK3R
KN4CK3R reviewed Jan 25, 2022
View reviewed changes
routers/web/home.go Outdated Show resolved Hide resolved
@danog
Copy link
Contributor Author

danog commented Jan 26, 2022

Cleaned up a bit :)

@danog
Copy link
Contributor Author

danog commented Jan 26, 2022

Marking this as draft, it seems Google doesn't support nested sitemap indexes, I'll have to rewrite this.

@danog danog marked this pull request as draft January 26, 2022 17:09
@danog danog marked this pull request as ready for review January 29, 2022 16:45
@danog
Copy link
Contributor Author

danog commented Jan 29, 2022 •
edited
Loading

All done, defaulting to the (spec limit) of 50k URLs per sitemap, wasn't sure whether to add another configuration value just for this.

6543
6543 reviewed Jan 30, 2022
View reviewed changes
routers/web/home.go Outdated Show resolved Hide resolved
Gusted
Gusted reviewed Jan 30, 2022
View reviewed changes
go.mod Outdated Show resolved Hide resolved
routers/web/home.go Outdated Show resolved Hide resolved
routers/web/web.go Outdated Show resolved Hide resolved
routers/web/home.go Outdated Show resolved Hide resolved
@a1012112796 a1012112796 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 5, 2022
@danog
Copy link
Contributor Author

danog commented Apr 30, 2022

All done!
Wrote a simplified sitemap library with additional checks as per the specs+tests.

6543
6543 reviewed Apr 30, 2022
View reviewed changes
go.mod Outdated Show resolved Hide resolved
@6543
Copy link
Member

6543 commented Apr 30, 2022

please do not force-push or squash, it will make review easyer & in the end it always get squash-merged 👍

@6543 6543 added this to the 1.17.0 milestone Apr 30, 2022
6543
6543 reviewed Apr 30, 2022
View reviewed changes
routers/web/home.go Outdated Show resolved Hide resolved
6543
6543 reviewed Apr 30, 2022
View reviewed changes
routers/web/home.go Outdated Show resolved Hide resolved
@lunny lunny modified the milestones: 1.17.0, 1.18.0 Jun 4, 2022
@Tha14
Copy link

Tha14 commented Jun 21, 2022

Any updates on this? I would really like to have sitemaps available. Thanks

Gusted
Gusted reviewed Jun 21, 2022
View reviewed changes
modules/sitemap/sitemap.go Outdated Show resolved Hide resolved
routers/web/home.go Outdated Show resolved Hide resolved
routers/web/home.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Jun 23, 2022
@danog danog requested a review from Gusted June 24, 2022 21:52
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Jun 25, 2022
@6543
Copy link
Member

6543 commented Jun 25, 2022

🚀

@6543 6543 merged commit 95383b7 into go-gitea:main Jun 25, 2022
zjjhot added a commit to zjjhot/gitea that referenced this pull request Jun 26, 2022
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
e4b7cf7 
* giteaofficial/main:
  Use neutral language in comments and docs (go-gitea#20135)
  Fix remove file on initial comment (go-gitea#20127)
  Add doctor command to write commit-graphs (go-gitea#20007)
  Add sitemap support (go-gitea#18407)
  Use new config options (go-gitea#20125)
  Remove depricated queue indexer usage (go-gitea#20124)
  Disable status checks in template if none found (go-gitea#20088)
  Fix typos related to ErrTaskDoesNotExist error (go-gitea#20118)
@danog
Copy link
Contributor Author

danog commented Jun 26, 2022

Ayy, thanks for merging 🎉
Do you think this could be backported/released in the upcoming 1.17?

@Gusted
Copy link
Contributor

Gusted commented Jun 26, 2022

We don't backport features.

@danog
Copy link
Contributor Author

danog commented Jun 26, 2022

We don't backport features.

Ah well, guess I'll wait for 1.18 :)

@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KN4CK3R KN4CK3R KN4CK3R left review comments

@6543 6543 6543 approved these changes

@Gusted Gusted Gusted approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.

Generate a sitemap for SEO
9 participants
@danog @codecov-commenter @6543 @Tha14 @Gusted @KN4CK3R @lunny @GiteaBot @a1012112796