Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

433: Display event publisher in event preview #457

Merged
merged 6 commits into from
Oct 30, 2019

Conversation

rBoost
Copy link
Contributor

@rBoost rBoost commented Oct 29, 2019

#433

Add publisher information under event actions and under event location for event page.

Added also a rest API client for fetching the publisher's information.
I also refactored and cleaned a bit the event page/event details code.

@rBoost rBoost requested review from Rikuoja and niglu1 October 29, 2019 10:42
Copy link
Contributor

@Rikuoja Rikuoja left a comment

Choose a reason for hiding this comment

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

Okay, so apparently a new HTTP API client (axios) was added here. What is the use case, i.e. what are the advantages of adding a new library vs. using fetch as is done with other API calls here? Do you suggest transitioning to the new library instead?

@rBoost
Copy link
Contributor Author

rBoost commented Oct 29, 2019

Okay, so apparently a new HTTP API client (axios) was added here. What is the use case, i.e. what are the advantages of adding a new library vs. using fetch as is done with other API calls here? Do you suggest transitioning to the new library instead?

Yeah, I would suggest we change all the existing fetch requests to use axios.
I have noticed in multiple projects that it makes life easier for developers compared to fetch.

Here's a blog post about some of the pros of using axios: https://blog.logrocket.com/axios-or-fetch-api/

@Rikuoja
Copy link
Contributor

Rikuoja commented Oct 29, 2019

Yeah, I would suggest we change all the existing fetch requests to use axios.
I have noticed in multiple projects that it makes life easier for developers compared to fetch.

Okay, can you give a ballpark figure how long the change should take? Probably not too long, since the function calls are rather similar anyway, or am I mistaken?

@rBoost
Copy link
Contributor Author

rBoost commented Oct 29, 2019

Yeah, I would suggest we change all the existing fetch requests to use axios.
I have noticed in multiple projects that it makes life easier for developers compared to fetch.

Okay, can you give a ballpark figure how long the change should take? Probably not too long, since the function calls are rather similar anyway, or am I mistaken?

It shouldn't take that long, few hours (2-3).

@Rikuoja
Copy link
Contributor

Rikuoja commented Oct 30, 2019

Yeah, I would suggest we change all the existing fetch requests to use axios.
I have noticed in multiple projects that it makes life easier for developers compared to fetch.

Okay, can you give a ballpark figure how long the change should take? Probably not too long, since the function calls are rather similar anyway, or am I mistaken?

It shouldn't take that long, few hours (2-3).

Great, I would recommend doing that sooner rather than later, otherwise it is forgotten and we end up with competing ways to call the API in the UI 👍

@Rikuoja Rikuoja merged commit f22c84a into master Oct 30, 2019
@Rikuoja Rikuoja deleted the feature/433-display-event-publisher-in-event-preview branch October 30, 2019 09:56
@Rikuoja
Copy link
Contributor

Rikuoja commented Oct 30, 2019

I made a new issue for the other API calls, #466

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants