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

Feature: make issues and github stuff work outside of CYF #773

Merged
merged 18 commits into from
Jul 17, 2024

Conversation

SallyMcGrath
Copy link
Member

@SallyMcGrath SallyMcGrath commented Jul 4, 2024

What does this change?

We had limited this to CYF's org because it felt like dangerous madness not to. But actually MCB need their own repos so I've removed the limit.

This reminded me of the problem with paginated issues when trainees start PRing to coursework repos -- the coursework gets pushed off page 1.

So I think I've paginated now? I need to test a bit more, hence draft PR

Common Theme?

  • Yes

This changes the way issues are pulled from github for everyone, which is why this PR is draft until I've tested a bit more.

it affects mainly

issues.html
fetch-paginated-issues.html <= new partial called recursively

The other changes are config changes to remove the old settings.

Note: this won't build! that's because MCB need to create all these repos or remove the links to them.

That's happening on this ticket Migracode-Barcelona/Table-of-Contents#1

On my local copy of MCB, I have just deleted all content except JS1, as I have created that test module to build this stuff.

Org Content?

I also made a logo just because it was bugging me a lot to look at

Checklist

Who needs to know about this?

MCB need some liberty
next I need to either solve pagination or hassle Daniel to make them a proxy...
and yes it works as expected
point to MCB
NOTE: the site will not build until all these repos exist,  see Migracode-Barcelona/Table-of-Contents#1
they don't know where their SVG is so I just faked it up on photopea
Copy link

netlify bot commented Jul 4, 2024

Deploy Preview for cyf-curriculum failed. Why did it fail? →

Name Link
🔨 Latest commit 3c0917f
🔍 Latest deploy log https://app.netlify.com/sites/cyf-curriculum/deploys/6697e6ae05dcf600083ccdf9

Copy link

netlify bot commented Jul 4, 2024

Deploy Preview for cyf-programming failed. Why did it fail? →

Name Link
🔨 Latest commit 3c0917f
🔍 Latest deploy log https://app.netlify.com/sites/cyf-programming/deploys/6697e6aebd477d000874d063

Copy link

netlify bot commented Jul 4, 2024

Deploy Preview for cyf-guides canceled.

Name Link
🔨 Latest commit 3c0917f
🔍 Latest deploy log https://app.netlify.com/sites/cyf-guides/deploys/6697e6aebfac6a00086b82c9

Copy link

netlify bot commented Jul 4, 2024

Deploy Preview for cyf-sdc failed. Why did it fail? →

Name Link
🔨 Latest commit 3c0917f
🔍 Latest deploy log https://app.netlify.com/sites/cyf-sdc/deploys/6697e6aee11aa500086a7258

[params]
# Don't use the paginating proxy, because it's slow.
# This means some issues will be missing when fetched.
issuesorgapi = "https://api.github.com/repos/Migracode-Barcelona/"
Copy link
Member Author

Choose a reason for hiding this comment

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

we can probably rm all this, though I did like just skipping pagination on local

@SallyMcGrath
Copy link
Member Author

I gott a test this a lot more, but of course am getting rate limited. Another strong argument for no crawling on dev!

Copy link
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

Generally LGTM except I'm not sure the pagination check is a reliable one - I'm pretty sure I saw partial responses still have nexts (probably because e.g. GitHub treats PRs as issues and maybe filters after it paginates or something)

common-theme/layouts/partials/fetch-paginated-issues.html Outdated Show resolved Hide resolved
getJSON is deprecated and GetRemote will soon have a way of getting more headers information, allowing us to replace the functionality of Daniel's proxy
@SallyMcGrath SallyMcGrath marked this pull request as ready for review July 5, 2024 16:38
@SallyMcGrath SallyMcGrath merged commit 4e1ae71 into main Jul 17, 2024
6 of 18 checks passed
@SallyMcGrath SallyMcGrath deleted the feature/mcb-point-to-new-org branch July 17, 2024 16:07
@SallyMcGrath SallyMcGrath restored the feature/mcb-point-to-new-org branch July 17, 2024 17:29
SallyMcGrath added a commit that referenced this pull request Jul 17, 2024
SallyMcGrath added a commit that referenced this pull request Jul 17, 2024
@SallyMcGrath SallyMcGrath deleted the feature/mcb-point-to-new-org branch August 17, 2024 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants