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

Hotfix/api parsing for half duration lectures #981

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

limwa
Copy link
Member

@limwa limwa commented Sep 29, 2023

This PR introduces a hotfix to a problem where the API parsing throws an exception when aula_duracao, in the API response, is a double (1.5 for a lecture that lasts for 1h30min). This was done by checking if the value is a double and, if so, multiply it by two and only then cast it to an integer, instead of converting the result to an integer.

Review checklist

  • Terms and conditions reflect the current change
  • Contains enough appropriate tests
  • If aimed at production, writes a new summary in whatsnew/whatsnew-pt-PT
  • Properly adds an entry in changelog.md with the change
  • If PR includes UI updates/additions, its description has screenshots
  • Behavior is as expected
  • Clean, well-structured code

@limwa
Copy link
Member Author

limwa commented Sep 29, 2023

@bdmendes should this PR target master as the base, since it is a hotfix? Or do we go for develop and then port the commit to master?

Also, does this PR require a whatsnew and a changelog entry? I am not entirely familiar with the process in uni.

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #981 (ba66472) into develop (60c7d5b) will increase coverage by 1%.
Report is 20 commits behind head on develop.
The diff coverage is 25%.

Additional details and impacted files
@@          Coverage Diff           @@
##           develop   #981   +/-   ##
======================================
+ Coverage       22%    23%   +1%     
======================================
  Files          143    143           
  Lines         4357   4349    -8     
======================================
+ Hits           948    964   +16     
+ Misses        3409   3385   -24     

@LuisDuarte1
Copy link
Member

LuisDuarte1 commented Sep 29, 2023

@bdmendes should this PR target master as the base, since it is a hotfix? Or do we go for develop and then port the commit to master?

You should change it into master, since it's a hotfix

Also, does this PR require a whatsnew and a changelog entry? I am not entirely familiar with the process in uni.

I think you don't need to do that

@bdmendes
Copy link
Member

We are migrating away from the changelog file in favor of automated releases soon, so no need. The whatsnew is usually generic for the stores description

LuisDuarte1
LuisDuarte1 previously approved these changes Sep 29, 2023
Copy link
Member

@LuisDuarte1 LuisDuarte1 left a comment

Choose a reason for hiding this comment

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

Lgtm, good work and nice catch, this was impossible to test for when we did the linting changes, change the base to master and I'll merge this

bdmendes
bdmendes previously approved these changes Sep 29, 2023
@bdmendes bdmendes changed the base branch from develop to master September 29, 2023 20:58
@bdmendes bdmendes dismissed stale reviews from LuisDuarte1 and themself September 29, 2023 20:58

The base branch was changed.

@bdmendes bdmendes merged commit 7558258 into master Sep 29, 2023
6 checks passed
@bdmendes bdmendes deleted the hotfix/api-parsing-for-half-duration-lectures branch September 29, 2023 20:59
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