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

添加Champion Rankings数据 #123

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

diaolingzc
Copy link

添加Champion Rankings数据
image

@cangzhang
Copy link
Owner

cangzhang commented Nov 30, 2021

This whole idea is good, thanks. But:

  1. op.gg is not always available for china mainland visitors, so this window may not work properly as expected.
  2. everything in this window is readonly, I don't think that makes sense.
  3. I don't think taking everything from op.gg (or other platforms) is a good thing. (maybe you can convince me about it.)

@diaolingzc
Copy link
Author

Regarding your three questions:

  1. the problem of op.gg access, I think it can be solved by data crawler, just like https://github.com/champ-r/data-crawler.
  2. Champion Rankings is essentially the statistical version of heroes. The nature of data aggregation itself is to let users know which heroes are the strongest in the current version.
  3. the method of data crawling can circumvent the project itself from obtaining data from op.gg.

@cangzhang cangzhang force-pushed the main branch 4 times, most recently from 8f96d2a to 9b92bbc Compare May 30, 2022 03:07
@cangzhang cangzhang force-pushed the main branch 7 times, most recently from 55e017c to 16d1d28 Compare June 11, 2022 03:35
@cangzhang cangzhang force-pushed the main branch 3 times, most recently from 7af7eff to 6c50b00 Compare July 22, 2022 04:41
@cangzhang cangzhang force-pushed the main branch 5 times, most recently from 050b917 to 91b99a5 Compare September 26, 2022 12:35
@cangzhang cangzhang force-pushed the main branch 3 times, most recently from 2eb4c94 to dcf85fd Compare October 9, 2022 17:07
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