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.

Sign up for GitHub

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

Jump to bottom

[Issue #6843] Remove use of some magic numbers #6853

Merged
merged 6 commits into from
Jan 26, 2021
Merged

[Issue #6843] Remove use of some magic numbers #6853

merged 6 commits into from
Jan 26, 2021

Conversation

burner761
Copy link
Contributor

this Request seeks to make the code more readable by reducing the use of magic numbers which can confuse people who want to edit the code in the future.

@cla-bot
Copy link

cla-bot bot commented Jan 21, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @DanieSimonlLowe on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

@lgtm-com
Copy link

lgtm-com bot commented Jan 21, 2021

This pull request introduces 3 alerts when merging 2a93b16 into c796bac - view on LGTM.com

new alerts:

  • 1 for Syntax error
  • 1 for Missing variable declaration
  • 1 for Result of integer division may be truncated

@burner761 burner761 force-pushed the danielLowe/6799-magic-numbers branch from 2a93b16 to 4fd1e8b Compare January 21, 2021 03:46
@cla-bot
Copy link

cla-bot bot commented Jan 21, 2021

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @DanieSimonlLowe on file. In order for us to review and merge your code, please contact the project maintainers to get yourself added.

mtnorthcott
mtnorthcott suggested changes Jan 21, 2021
View reviewed changes
Copy link
Contributor

@mtnorthcott mtnorthcott left a comment

Choose a reason for hiding this comment

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

Mainly style changes.

@lgtm-com
Copy link

lgtm-com bot commented Jan 21, 2021

This pull request introduces 2 alerts when merging 4fd1e8b into c796bac - view on LGTM.com

new alerts:

  • 1 for Missing variable declaration
  • 1 for Result of integer division may be truncated

@burner761 burner761 force-pushed the danielLowe/6799-magic-numbers branch from 4fd1e8b to 63ba3bf Compare January 21, 2021 21:23
@cla-bot cla-bot bot added the cla-signed CLA Bot: community license agreement signed label Jan 21, 2021
@mtnorthcott mtnorthcott changed the title [#6799] removed use of some magic numbers [Issue #6843 Remove use of some magic numbers Jan 21, 2021
@mtnorthcott mtnorthcott changed the title [Issue #6843 Remove use of some magic numbers [Issue #6843] Remove use of some magic numbers Jan 21, 2021
@t-book
Copy link
Contributor

t-book commented Jan 26, 2021

@mtnorthcott This looks good for me, ready for merge from your side?

@mtnorthcott
Copy link
Contributor

@t-book Looks good to me

@t-book t-book added this to the 3.2 milestone Jan 26, 2021
@t-book
Copy link
Contributor

t-book commented Jan 26, 2021

nice thanks @mtnorthcott

  • Code looks good to me
  • Tested live, no regression spotted.

Merged.

@t-book t-book merged commit 2a8786c into GeoNode:master Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mtnorthcott mtnorthcott mtnorthcott approved these changes

Assignees
No one assigned
Labels
cla-signed CLA Bot: community license agreement signed
Projects
None yet
Milestone
3.2
Development

Successfully merging this pull request may close these issues.
3 participants
@burner761 @t-book @mtnorthcott