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

[GithubTrendingBridge] Add bridge #1492

Merged
merged 8 commits into from
Apr 3, 2020
Merged

[GithubTrendingBridge] Add bridge #1492

merged 8 commits into from
Apr 3, 2020

Conversation

liamka
Copy link
Contributor

@liamka liamka commented Mar 16, 2020

No description provided.

@AntoineTurmel
Copy link
Contributor

nice !!

@somini
Copy link
Contributor

somini commented Mar 18, 2020

Thanks. Found some small issues:

  • The URI is invalid. It should be https://github.com/$WHATEVER_REPO. See the usage of BASE_URL on other bridges.
  • Error when fetching the description:
Notice: Trying to get property 'innertext' of non-object in rss-bridge/bridges/GithubTrendingBridge.php on line 49
  • If you don't use the timestamp, remove it.

@liamka
Copy link
Contributor Author

liamka commented Mar 20, 2020

Fixed issues!

@somini
Copy link
Contributor

somini commented Mar 23, 2020

That is a metric ton of languages. I predict that this list can get stale fast. It would be acceptable to have a free-form input.

I haven't tested this yet, but from a cursory review this sounds acceptable.

@somini
Copy link
Contributor

somini commented Mar 24, 2020

LGTM.

@liamka
Copy link
Contributor Author

liamka commented Apr 2, 2020

@somini can you add this badge in next release?

@somini
Copy link
Contributor

somini commented Apr 2, 2020

I'm just a regular RSS Bridge user, I don't have special powers. I just review new bridges that appear.

You want anyone from @RSS-Bridge

@liamka
Copy link
Contributor Author

liamka commented Apr 2, 2020

@ArthurHoaro can you add?

@teromene
Copy link
Member

teromene commented Apr 2, 2020

I will merge this ASAP

@teromene teromene merged commit 413ae3c into RSS-Bridge:master Apr 3, 2020
@teromene
Copy link
Member

teromene commented Apr 3, 2020

Thank you for the bridge and sorry for the delay ! I am afraid that I have been extremely busy these weeks

infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
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.

4 participants