Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Start using tox #36

Merged
merged 2 commits into from
Feb 18, 2022
Merged

Start using tox #36

merged 2 commits into from
Feb 18, 2022

Conversation

sechkova
Copy link
Contributor

@sechkova sechkova commented Feb 8, 2022

Fixes: #34

Improve project structure:

  • Renamed the main source code path from ml-conversational-analytic-tool to mcat in order to make it a package. Long package names are discouraged and dashes are invalid symbols (see PEP8).
  • Moved tests out of the mcat package and made it a package too. Updated import paths.

Start using tox

  • Use tox for managing different python virtual environments and running tests. By integrating tox in the CI, the python environment in the CI is reproducible locally.

The new CI martx runs should be visible in this PR.

Copy link
Contributor

@annajung annajung left a comment

Choose a reason for hiding this comment

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

overall lgtm, just one nit comment

README.md Outdated
python -m unittest tests/<file_name>
```
8. By using tox
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I don't think the testing section needs to be numbered as it indicates steps instead of alternatives. could we use bullet points instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@difince difince left a comment

Choose a reason for hiding this comment

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

gltm

@sechkova
Copy link
Contributor Author

sechkova commented Feb 9, 2022

Only the README is updated to replace a numbered list with bullet points.

@annajung
Copy link
Contributor

annajung commented Feb 9, 2022

@sechkova I think you might have to fetch the latest and rebase to get the last check to pass.

Renamed the main source code path from ml-conversational-analytic-tool
to mcat in order to make it a package. Long package names are discouraged
and dashes are invalid symbols (see PEP8).

Moved tests out of the mcat package and made it a package too. Updated
import paths.

Updated the auto-generated docs and the README.

Signed-off-by: Teodora Sechkova <[email protected]>
Use tox for managing different python virtual environments
and running tests. By integrating tox in the CI, the python
environment in the CI is reproducible locally.

Update the README.

Signed-off-by: Teodora Sechkova <[email protected]>
@sechkova
Copy link
Contributor Author

@sechkova I think you might have to fetch the latest and rebase to get the last check to pass.

Yeah, I forgot to do that, now it should be ok.
I don't know if additional settings are needed to make the new actions required or it GitHub will be smart enough to figure it out when merged. Let's wait and see.

@annajung annajung enabled auto-merge (squash) February 18, 2022 17:30
@annajung annajung disabled auto-merge February 18, 2022 17:39
@annajung annajung merged commit 41779a7 into vmware-archive:main Feb 18, 2022
@sechkova sechkova deleted the add-tox branch February 21, 2022 14:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Starting using tox
4 participants