Skip to content
This repository has been archived by the owner on Jul 2, 2024. It is now read-only.

[Solved] Check vowels #93 #116

Merged

Conversation

Rishitha-VasiReddy
Copy link
Contributor

Created a PR for this issue. Please review

@shorodilov shorodilov changed the base branch from master to devel November 24, 2023 11:41
@shorodilov
Copy link
Member

I have changed the base branch to devel.
We are following GitFlow approach within this project.
master branch contains released features only, and the devel is used to accumulate changes while development.

@shorodilov shorodilov self-requested a review November 24, 2023 11:42
Copy link
Member

@shorodilov shorodilov 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 to me, nice work @Rishitha-VasiReddy.

I need to check the reason the tests are failed, and we can merge it once its fixed.

src/vowels/checkvowels.py Show resolved Hide resolved
Copy link
Member

@shorodilov shorodilov left a comment

Choose a reason for hiding this comment

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

Found the reason the tests are failed.

Just fix imports in test_vowels, and we will be ready to merge changes into devel.

Nice work, thanks @Rishitha-VasiReddy.

tests/vowels/test_checkvowels.py Outdated Show resolved Hide resolved
@shorodilov shorodilov linked an issue Nov 24, 2023 that may be closed by this pull request
@Rishitha-VasiReddy
Copy link
Contributor Author

I've made the necessary changes. Please review

Added docstrings
@shorodilov shorodilov self-requested a review November 25, 2023 14:50
@shorodilov
Copy link
Member

Nice.

Need to check the reason tests are failed.
I'll come back to this PR in morning (EEST).

Copy link
Member

@shorodilov shorodilov left a comment

Choose a reason for hiding this comment

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

The reason "tox runner" fails testing - the vowels package is not included to the list of packages to install.
It's not a big deal, I will re-structure code base on pre-release.
Good job.
LGTM.

image
image

@shorodilov shorodilov merged commit b44840d into edu-python-course:devel Nov 27, 2023
2 of 12 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check vowels
2 participants