Skip to content

News endpoint was done, model, API and test were created#37

Open
davidfmb93 wants to merge 8 commits intomainfrom
david-sentiment-news
Open

News endpoint was done, model, API and test were created#37
davidfmb93 wants to merge 8 commits intomainfrom
david-sentiment-news

Conversation

@davidfmb93
Copy link
Copy Markdown

No description provided.

Comment thread Polygon.Net.Tests/FunctionalTests/NewsTests.cs
Comment thread Polygon.Net/API/NewsApi.cs Outdated
Comment thread Polygon.Net/API/NewsApi.cs Outdated
Comment thread Polygon.Net/API/NewsApi.cs Outdated
Comment thread Polygon.Net/Models/NewsInfo.cs Outdated
mishyjari
mishyjari previously approved these changes Mar 6, 2023
Copy link
Copy Markdown
Contributor

@mishyjari mishyjari left a comment

Choose a reason for hiding this comment

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

lgtm

…d(PublishUtc,HashNextUrl). Furthermore, new tests and methods services have been created
Comment thread Polygon.Net/API/NewsApi.cs Outdated

if(nextPage != null) {
queryParams.Add("cursor", nextPage);
}else {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Try to use CodeMaid to clean the code formatting, this should be in different lines

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Code has been cleaned up

Comment thread Polygon.Net/API/NewsApi.cs Outdated
/// <param name="limit">Limit the number of results returned, default is 10 and max is 1000</param>
/// <param name="sort">Sort field used for ordering</param>
/// <returns>NewsResponse</returns>
public async Task<NewsResponse> GetTodayNewsAsync(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is not necesarry we can do it in the az func, only passing the current date as start and end.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The function has been removed
Fixed it

Comment thread Polygon.Net/API/NewsApi.cs Outdated
/// Get the next page of Polygon news.
/// </summary>
/// <param name="nextPage">hash of the next page</param>
public async Task<NewsResponse> GetNextPageNewsAsync(string nextPage = null){
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't know what do you need this?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The function has been removed
Fixed it

Comment thread Polygon.Net/API/NewsApi.cs Outdated
DateTime? endTime = default,
string ticker = null,
string order = null,
int? limit = null,
Copy link
Copy Markdown
Contributor

@estebanl5 estebanl5 Mar 7, 2023

Choose a reason for hiding this comment

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

You can avoid the null here

You need to add nullable property to the csproj file
image

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I enable this property but need to initialize all variables to null.

Task<PolygonResponse<TickerType>> GetTickerTypesAsync(string assetClass = default, string locale = default);

Task<NewsResponse> GetNewsAsync(DateTime? startTime = default, DateTime? endTime = default, string ticker = null, string order = null, int? limit = null, string sort = null);
Task<NewsResponse> GetNewsAsync(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

My suggestion is to keep only one and remove the GetTodayNewsAsync and GetNextPageNewsAsync

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The function removed
Fixed it

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Mar 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 29 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

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.

3 participants