search

UQComputingSociety / uqcsbot-slack

:mortar_board: UQCSbot: our friendly little Slack bot
https://slack.uqcs.org.au
MIT License
55 stars 44 forks source link

Feature: !techcrunch command which posts a message with 5 top-most TechCrunch articles in the Latest News section #562

Closed gaenrique closed 3 years ago

gaenrique commented 3 years ago

This command webscrapes techcrunch.com and posts a message containing the top 5 articles in the Latest News section. The message which the bot sends also makes the headlines clickable (linking to the article).

I believe that if people enjoy this command, it would be a nice bot command to run automatically once a day.

gaenrique commented 3 years ago

@nicklambourne I have not had a look at that. I'll make sure to give it a go when I have time. Thanks for the feedback

gaenrique commented 3 years ago

Jenkins seems to be failing since build 795. I don't know what the problem is.

nicklambourne commented 3 years ago

uqcsbot/scripts/news.py:6:1: F401 'requests' imported but unused uqcsbot/scripts/news.py:7:1: F401 'bs4.BeautifulSoup' imported but unused uqcsbot/scripts/news.py:14:42: W291 trailing whitespace uqcsbot/scripts/news.py:18:31: E203 whitespace before ':' uqcsbot/scripts/news.py:18:61: E203 whitespace before ':' uqcsbot/scripts/news.py:24:55: W291 trailing whitespace uqcsbot/scripts/news.py:39:1: W293 blank line contains whitespace

gaenrique commented 3 years ago

Thanks! I didn't how to find the errors but I found them now.

gaenrique commented 3 years ago

Alright now for some reason the trivia tests fail even though I didn't touch them. I'll try and find out what is wrong sorry

nicklambourne commented 3 years ago

There's a good chance that's not your fault, some of our tests are flakey (and by ours I mostly mean Bradley).

nicklambourne commented 3 years ago

Also, no need to be sorry. I for one am thrilled someone's working on the bot again. The CI checks exist so we know what's wrong, they exist independently of the bot actually running in Slack, so there's no issue if they fail during development.

gaenrique commented 3 years ago

I appreciate the support. First time contributing to open source so I'm new to all this

gaenrique commented 3 years ago

One thing is for sure though. I do not understand this trailing whitespace error

gaenrique commented 3 years ago

Weird random errors keep occurring so I might just give it a break and try again later

nicklambourne commented 3 years ago

@bradleysigma unless you can fix those tests I'm going to remove !yelling.

nicklambourne commented 3 years ago

@gaenrique Give us a day or so to sort things out with Jenkins, we'll get it merged, I'm just at work at the moment.

nicklambourne commented 3 years ago

@kentonlam are you still technical czar?

gaenrique commented 3 years ago

@nicklambourne sounds good. I'll just give it one more shot as the last error was just an over-indentation on my file

bradleysigma commented 3 years ago

@bradleysigma unless you can fix those tests I'm going to remove !yelling.

Remove the tests or remove the feature?

gaenrique commented 3 years ago

Changes have been implemented. Feel free to give any additional feedback if you would like anything else changed :)

nicklambourne commented 3 years ago

@bradleysigma the feature. You may remove the offending tests as a half-measure if you so choose.

gaenrique commented 3 years ago

@nicklambourne I actually just realised that I forgot to add an error message if the RSS feed can't be accessed so I'll just finish that up real quick

gaenrique commented 3 years ago

Thanks for fixing the style! You can merge it whenever you're ready.

Final questions since I'm new to all this. So now that it is all approved, do I merge it or do you guys have to merge?

katrinafyi commented 3 years ago

Fairly certain we have to merge because of permissions. (I forgot about that 😅)