Adds Vapor backend to project#109
Conversation
unnamedd
left a comment
unnamedd
left a comment
There was a problem hiding this comment.
@loloop, thanks for the effort here. As I am not working—not even a bit—in the CH projects, I do not see myself in a position to block your work with a "Request changes" from GitHub's Pull Request; however, I’m raising some points here for your appreciation.
Give a check, and if you think they are addressable, apply the comments and, if you want me to give you a second review, just re-request a review and I will do so with great pleasure.
| guard | ||
| let url = URL(string: meetup.url), | ||
| case (let data, _) = try await URLSession.shared.data(from: url), | ||
| let html = String(data: data, encoding: .utf8) | ||
| else { | ||
| throw Error.urlError | ||
| } |
There was a problem hiding this comment.
I suggest you to split into two different guards here. You are not being clear enough when raising errors.
If something goes wrong with the URL, then you will throw Error.urlError, however, in case you get an error while you are requesting the content from that URL using URLSession or get an error while you are converting from the received data into a String, you will also throw Error.urlError. You had to separate at least in two more guards to throw the proper error.
| throw Error.urlError | ||
| } | ||
|
|
||
| guard !url.absoluteString.contains("Entrar") else { |
There was a problem hiding this comment.
is "Entrar" a safe string to use?
There was a problem hiding this comment.
Used to be, but meetup has released a completely new design while I was working on moving this code around, so I'll need to do more testing to figure out if everything here still works
| case .addressError: | ||
| "Erro ao buscar o endereço" | ||
| case .coordinateError: | ||
| "Erro no parse das coordenadas" |
There was a problem hiding this comment.
If we are going to use the error messages in portuguese, perhaps here we should use "extrair" instead of "parse". What do you think?
| "Erro no parse das coordenadas" | |
| "Erro ao extrair as coordenadas" |
There was a problem hiding this comment.
These errors used to be part of the UI when SwiftSoup was directly integrated in the app. I'm totally down to translate these messages to english now that they're part of the API
| case .coordinateError: | ||
| "Erro no parse das coordenadas" | ||
| case .dateError: | ||
| "Não consegui encontrar ou fazer decode da data" |
There was a problem hiding this comment.
| "Não consegui encontrar ou fazer decode da data" | |
| "Não foi possível encontrar ou decodificar a data" |
|
|
||
| extension String { | ||
| fileprivate func removingFirstOccurrence(of word: String) -> String { | ||
| guard let range = range(of: "\\b\(word)\\b", options: [.regularExpression, .caseInsensitive]) else { |
There was a problem hiding this comment.
Here, we have just one use case, but I was wondering if would be a good place to make use of RegexBuilder here
| // Created by Mauricio on 5/14/25. | ||
| // | ||
|
|
||
| import CocoaHeadsCore |
There was a problem hiding this comment.
CocoaHeadsKit or CocoaHeadsCore? I see them being mixed here.
There was a problem hiding this comment.
CocoaHeadsKit is the package that provides all the code for the apps, CocoaHeadsCore is a new package that provides shared code between the apps and the Vapor backend
There was a problem hiding this comment.
perhaps it would be better if they were renamed in order to avoid confusion. To me, as you probably noticed, they were just getting mixed with each other, while in fact, they do separate things, but I am completely out of ideas to suggest something better haha
There was a problem hiding this comment.
I think better documenting them would help with this
loloop
changed the title
|
@unnamedd I know it's been a while, but can you check the PR again? Most of what you reviewed before was scrapped due to a change in how meetup works in favor of adopting an AI scraping provider |
|
Yeah sure, @loloop! Will do it! 😉 |
|
@loloop sorry mate, I completely forgot about your request here! I will give a check still this week! ;) |
2 participants
This PR intends to:
• Add a vapor backend starter base to the project
• Remove the SwiftSoup dependency for meetup fetching from the app, delegating that feature to the backend instead
Tasks:
Next steps