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

Update luogu.ts #21

Closed
wants to merge 1 commit into from
Closed

Conversation

LuohanXiao
Copy link

@wxh06 wxh06 requested review from weibohan07 and wxh06 June 10, 2024 16:54
Copy link
Member

@wxh06 wxh06 left a comment

Choose a reason for hiding this comment

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

请确保自己知道自己在修改什么,不要仅仅简单地进行全文替换来水 PR。

export const judgementUrl = "https://www.luogu.com.cn/judgement";
export const judgementUrl = "https://www.luogu.com/judgement";
Copy link
Member

Choose a reason for hiding this comment

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

https://www.luogu.com/judgement 能正常访问吗?

url.hostname === "www.luogu.com.cn" && isHTTPorHTTPS(url);
url.hostname === "www.luogu.com" && isHTTPorHTTPS(url);
Copy link
Member

Choose a reason for hiding this comment

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

这个函数的作用是判断 URL 是否为洛谷内链,这么修改是否不妥?

Copy link
Member

@weibohan07 weibohan07 left a comment

Choose a reason for hiding this comment

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

The changes you made is just simply replace all legacy luogu domains into new domains in our viewer. It's not hard to guess why you haven't also changed these domains in our archive module. The only possible reason I guess is that you are completely unfamiliar to our project. Thus I suggest you carefully read throughout our codes, and then you may pr again.

@wxh06
Copy link
Member

wxh06 commented Jun 18, 2024

Fixed in #22.

@wxh06 wxh06 closed this Jun 18, 2024
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.

3 participants