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

iox-#536 refactored creation/getter method names in units::Duration #546

Merged
merged 8 commits into from
Feb 5, 2021
Merged

iox-#536 refactored creation/getter method names in units::Duration #546

merged 8 commits into from
Feb 5, 2021

Conversation

enkeyz
Copy link
Contributor

@enkeyz enkeyz commented Feb 5, 2021

Signed-off-by: enkeyz klarnorbert@gmail.com

Pre-Review Checklist for the PR Author

  1. Branch follows the naming format (iox-#123-this-is-a-branch)
  2. Commits messages are according to this guideline
    • Commit messages have the issue ID (iox-#123 commit text)
    • Commit messages are signed (git commit -s)
    • Commit author matches Eclipse Contributor Agreement (and ECA is signed)
  3. Update the PR title
    • Follow the same conventions as for commit messages
    • Link to the relevant issue
  4. Relevant issues are linked
  5. Add sensible notes for the reviewer
  6. All checks have passed
  7. Assign PR to reviewer

Notes for Reviewer

Refactored creation/getter method names to be more meaningful for "units::Duration". Partly closes: #536

Checklist for the PR Reviewer

  • Commits are properly organized and messages are according to the guideline
  • Code according to our coding style and naming conventions
  • Unit tests have been written for new behavior
  • Public API changes are documented via doxygen
  • Copyright owner are updated in the changed files
  • PR title describes the changes

Post-review Checklist for the PR Author

  1. All open points are addressed and tracked via issues

Post-review Checklist for the Eclipse Committer

  1. All checkboxes in the PR checklist are checked or crossed out
  2. Merge

References

  • Closes TBD

…o be more meaningful

Signed-off-by: enkeyz <klarnorbert@gmail.com>
@dkroenke
Copy link
Member

dkroenke commented Feb 5, 2021

@enkeyz There is one requirement from the eclipse organization to sign the Eclipse Contributor Agreement, this can be done in your eclipse account.

@enkeyz
Copy link
Contributor Author

enkeyz commented Feb 5, 2021

@enkeyz There is one requirement from the eclipse organization to sign the Eclipse Contributor Agreement, this can be done in your eclipse account.

Just updated. Should be okey.

@dkroenke dkroenke added the refactoring Refactor code without adding features label Feb 5, 2021
@dkroenke
Copy link
Member

dkroenke commented Feb 5, 2021

@enkeyz there is a merge conflict and the CI doesn't start then. Can you merge the latest master in your branch?

@enkeyz
Copy link
Contributor Author

enkeyz commented Feb 5, 2021

@enkeyz there is a merge conflict and the CI doesn't start then. Can you merge the latest master in your branch?

Done.

Copy link
Contributor

@mossmaurice mossmaurice left a comment

Choose a reason for hiding this comment

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

Thanks for you contribution! The codes changes look great. However, I've noticed two workflow-related points:

  1. Last two commits are missing "iox-Improving units::Duration #536" tag, this is needed for traceability. Could you rebase and force push?
  2. Could you link the issue Improving units::Duration #536 at the very end of your pull request description e.g. with a "Partly closes Improving units::Duration #536"?

Signed-off-by: enkeyz <klarnorbert@gmail.com>
Signed-off-by: enkeyz <klarnorbert@gmail.com>
Signed-off-by: enkeyz <klarnorbert@gmail.com>
Signed-off-by: enkeyz <klarnorbert@gmail.com>
Signed-off-by: enkeyz <klarnorbert@gmail.com>
Signed-off-by: enkeyz <klarnorbert@gmail.com>
@enkeyz
Copy link
Contributor Author

enkeyz commented Feb 5, 2021

Thanks for you contribution! The codes changes look great. However, I've noticed two workflow-related points:

  1. Last two commits are missing "iox-Improving units::Duration #536" tag, this is needed for traceability. Could you rebase and force push?
  2. Could you link the issue Improving units::Duration #536 at the very end of your pull request description e.g. with a "Partly closes Improving units::Duration #536"?

Fixed.

Copy link
Member

@elBoberido elBoberido left a comment

Choose a reason for hiding this comment

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

Could you clean up the duration usage in c_wait_set.cpp from my last comment. It seems it can be simplified even more.
After that this could be merged from my point of view.

Signed-off-by: enkeyz <klarnorbert@gmail.com>
@elBoberido
Copy link
Member

btw. thanks for your patience and contribution. It's highly appreciated :)

Copy link
Contributor

@mossmaurice mossmaurice left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@elBoberido elBoberido merged commit 871e0f4 into eclipse-iceoryx:master Feb 5, 2021
@enkeyz
Copy link
Contributor Author

enkeyz commented Feb 5, 2021

Thanks for the opportunity! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Refactor code without adding features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improving units::Duration
5 participants