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

docs: Extend contribution parts with DoD hints #94

Merged

Conversation

Mcthomas777
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Jul 13, 2022

Codecov Report

Merging #94 (66b0e76) into master (3d7b683) will increase coverage by 1.60%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master      #94      +/-   ##
==========================================
+ Coverage   93.09%   94.70%   +1.60%     
==========================================
  Files          69       68       -1     
  Lines        4955     4909      -46     
==========================================
+ Hits         4613     4649      +36     
+ Misses        342      260      -82     
Impacted Files Coverage Δ
src/pykiso/interfaces/simple_auxiliary.py 97.36% <0.00%> (-2.64%) ⬇️
src/pykiso/lib/robot_framework/uds_auxiliary.py 100.00% <0.00%> (ø)
...ykiso/lib/auxiliaries/udsaux/common/uds_request.py 100.00% <0.00%> (ø)
src/pykiso/lib/auxiliaries/acroname_auxiliary.py
...kiso/lib/connectors/cc_socket_can/cc_socket_can.py 98.73% <0.00%> (+0.01%) ⬆️
src/pykiso/lib/connectors/cc_rtt_segger.py 96.31% <0.00%> (+0.02%) ⬆️
src/pykiso/lib/auxiliaries/record_auxiliary.py 97.40% <0.00%> (+0.02%) ⬆️
src/pykiso/lib/auxiliaries/udsaux/uds_auxiliary.py 89.52% <0.00%> (+1.74%) ⬆️
..._control_auxiliary/instrument_control_auxiliary.py 96.59% <0.00%> (+2.27%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d7b683...66b0e76. Read the comment docs.

Copy link
Contributor

@sebastianpfischer sebastianpfischer 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!

Comment on lines 67 to 69
In order to maintain clear and user-friendly project make sure, that your
changes respect PEP8 standards. PEP8 is a guide that provides Python best
practices (naming, indentation,...).
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In order to maintain clear and user-friendly project make sure, that your
changes respect PEP8 standards. PEP8 is a guide that provides Python best
practices (naming, indentation,...).
PEP8 is the python naming convention and it also applies to our project.

:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's actually not just a naming convention, it also concerns white space, redundant syntax, indent, not sure I have to change this one

Copy link
Contributor

@Pog3k Pog3k Jul 18, 2022

Choose a reason for hiding this comment

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

To be exact PEP8 is just the python style guide for python code (:
Origin text is acutally ok..
If you want to improve
" PEP8 is the python style guide for python code that provides..... "

docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-contribution-doc-with-DoD-hints branch from 4458b4d to 6d18679 Compare July 15, 2022 13:21
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-contribution-doc-with-DoD-hints branch from 6d18679 to 80eeb53 Compare July 18, 2022 08:10
Copy link
Contributor

@Pog3k Pog3k left a comment

Choose a reason for hiding this comment

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

see suggestions

some_int_list_param: List[int], some_imported_type_param: namedtuple
) -> list:


Copy link
Contributor

@Pog3k Pog3k Jul 18, 2022

Choose a reason for hiding this comment

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

To prevent circular dependencies

Edit: To see the usage of future.annotations :)

Suggested change
.. code:: python
from __future__ import annotations
from typing import TYPE_CHECKING
if TYPE_CHECKING:
from .fruits import Banana
class BananaJuiceMaker:
def __init__(self, banana: Banana) -> None:
self.banana = banana
@classmethod
def juice_factory(cls, fruit: Union[Banana, Kiwi]) -> BananaJuiceMaker
return cls(fruit)
...

docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Show resolved Hide resolved
docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-contribution-doc-with-DoD-hints branch from 80eeb53 to e965ea5 Compare July 18, 2022 13:18
- pre-commit hook :
hook scripts that lint the added code using
flake8 and format it using black and isort (for
installation see :ref:`getting_started`)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the reference is missing in getting_started.rst (.. _getting_started)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not work neither with one or the other implemetation, just will replace with no reference

docs/getting_started/contribution_guide.rst Outdated Show resolved Hide resolved
@Mcthomas777 Mcthomas777 force-pushed the Extend-contribution-doc-with-DoD-hints branch from e965ea5 to 66b0e76 Compare July 19, 2022 08:54
@Pog3k Pog3k merged commit b799925 into eclipse:master Aug 1, 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.

5 participants