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

made variable public #2647

Merged
merged 2 commits into from
May 30, 2024
Merged

Conversation

h4l0gen
Copy link
Contributor

@h4l0gen h4l0gen commented May 28, 2024

Description of the changes being introduced by the pull request:
It makes _DEAFULT_PAYLOAD_TYPE variable public, so that flake8 linter don't show any error.

Fixes #2598

Signed-off-by: h4l0gen <ks3913688@gmail.com>
@h4l0gen
Copy link
Contributor Author

h4l0gen commented May 28, 2024

I have two choices for this:

  • Simply remove underscore from variable to make it public.
  • Use @Property

I used first option here, as i can see in codebase there is no special constraint on using DEFAULT_PAYLOAD_TYPE variable, so there is no meaning of using @Property. it will just make things complex.

Please tell me your thoughts?

side note flake8 don't show any errors now. see screenshot below
.

image

Signed-off-by: Kapil Sharma <ks3913688@gmail.com>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 9269100973

Details

  • 3 of 3 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 97.573%

Totals Coverage Status
Change from base Build 9264477504: 0.0%
Covered Lines: 1449
Relevant Lines: 1474

💛 - Coveralls

Copy link
Member

@jku jku left a comment

Choose a reason for hiding this comment

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

lgtm

@h4l0gen
Copy link
Contributor Author

h4l0gen commented May 30, 2024

Hi @jku, If it looks good to you, can we approve and merge this?

Thank you👍

@jku jku merged commit ac981a6 into theupdateframework:develop May 30, 2024
15 checks passed
@h4l0gen h4l0gen deleted the make_variable_publlic branch May 30, 2024 09:26
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.

SimpleEnvelope._DEFAULT_PAYLOAD_TYPE should be public?
3 participants