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

MAINT: remove hard coded event type class name. #46

Merged
merged 2 commits into from
Feb 23, 2022

Conversation

EmanIsmail
Copy link
Contributor

@EmanIsmail EmanIsmail commented Feb 13, 2022

Applicable Issues

fixes: #45

Description of the Change

Replace using hard coded event type class name with the builtin attribute __class__.__name__ or __qualname__.

Alternate Designs

N/A

Benefits

Easier maintenance in case changing

Possible Drawbacks

N/A

Sign-off

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or

(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or

(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.

(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.

Signed-off-by: Eman Ismail [email protected]

@EmanIsmail EmanIsmail requested a review from a team as a code owner February 13, 2022 13:56
@EmanIsmail EmanIsmail requested review from t-persson and fredjn and removed request for a team February 13, 2022 13:56
@t-persson
Copy link
Collaborator

Thank you for this pull request!
We'll look at this as soon as we can.
Two quick thing that can be solved before that.

In your description of this pull request you can add fixes: #45 under the Applicable Issues header. That way the issue will automatically close when we merge this.

Secondly, please write your full name and email in the Signed-off-by field in the description. For example, I sign off with this: Signed-off-by: Tobias Persson <[email protected]>

Thank you!

Copy link
Collaborator

@t-persson t-persson left a comment

Choose a reason for hiding this comment

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

Looks good.

I added a comment that you do not need to fix. We'll discuss this between maintainers. Might be that have to make a change when we've discussed it.

One more thing: You'll need to update the licenses at the top of each file as per: https://github.com/eiffel-community/.github/blob/master/CONTRIBUTING.md#license-management

In other words. Update to: Copyright 2019-2022 Axis Communications AB and others. at the very top :)

@@ -174,7 +174,7 @@ class EiffelBaseEvent(object):
tag = "_"
domain_id = "_"
version = "0.0.1"
meta = EiffelBaseMeta("EiffelBaseEvent", version)
meta = EiffelBaseMeta(__qualname__, version)
Copy link
Collaborator

Choose a reason for hiding this comment

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

__qualname__ will break our python2 compatibility, which can be considered fine if we do a major tag update.
But we will have to discuss whether or not we are okay with this first.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be fine with doing a major version step and effectively leaving python 2.x behind, although I think we need to hold of and provide any potential users with a deprecation notice/period. What I'm reluctant to do is to maintain both a python 2.x and a python 3.x versions, but that is mostly common sense given the EOL of Python 2.x.

@t-persson
Copy link
Collaborator

We've discussed this and it will be released in a major tag update. Thank you for your contribution

@t-persson t-persson merged commit cdee1f8 into eiffel-community:main Feb 23, 2022
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.

replace hardcoded event type name with __class__.__name__ for event class
3 participants