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

Iam 160 create charmed operator #2

Merged
merged 18 commits into from
Mar 23, 2023
Merged

Conversation

bencekov
Copy link
Contributor

@bencekov bencekov commented Mar 16, 2023

I rectified most comments. I have not included the changes, where I left a comment detailing why. If this PR is approved, I'll delete the previous one.

Updates not done:

  • Didn't take out function that returns service url. I use it while developing the integrations.
  • Didn't remove port var from config. This is consistent with application where the working port is configurable with envvar.
  • Took out integration test code, and integration test environmet from tox.ini. I'm not done with the integration tests, I'll include them with the pr including the relations.

@bencekov bencekov requested review from nsklikas and a team March 16, 2023 04:56
Copy link
Contributor

@nsklikas nsklikas left a comment

Choose a reason for hiding this comment

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

Please fix the linting errors and the github workflows.

tests/unit/__init__.py Outdated Show resolved Hide resolved
.github/workflows/unittest.yaml Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
@natalian98
Copy link
Contributor

you can add the publish action now - it should only run on push, so when this PR will be approved and merged

README.md Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
@natalian98
Copy link
Contributor

natalian98 commented Mar 17, 2023

Could you update pyproject.toml to reflect settings from other repos? Like here. I think it may solve some of the lint errors that we chose to ignore.
For example, current pyproject.toml specifies e.g. line length for ruff, but you are using isort and flake8. This is why you get a lot of errors saying your line should not exceed the default 79 characters and they don't disappear after you run black - it aims for 99 characters. With this file updated, the respected length will be 99 chars.

bencekov added a commit that referenced this pull request Mar 20, 2023
pyproject.toml Outdated Show resolved Hide resolved
lint-requirements.txt Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tox.ini Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
config.yaml Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
tests/unit/test_charm.py Outdated Show resolved Hide resolved
bencekov added a commit that referenced this pull request Mar 20, 2023
bencekov added a commit that referenced this pull request Mar 22, 2023
…ctions from charm.py. Changed Usage section in README.md
CONTRIBUTING.md Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
src/charm.py Show resolved Hide resolved
tests/unit/conftest.py Show resolved Hide resolved
src/charm.py Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Outdated Show resolved Hide resolved
bencekov added a commit that referenced this pull request Mar 22, 2023
…t in charm.py. Corrected documentation in CONTRIBUTING.md
natalian98
natalian98 previously approved these changes Mar 22, 2023
Copy link
Contributor

@natalian98 natalian98 left a comment

Choose a reason for hiding this comment

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

lgtm

nsklikas
nsklikas previously approved these changes Mar 22, 2023
tests/unit/test_charm.py Outdated Show resolved Hide resolved
tests/unit/test_charm.py Outdated Show resolved Hide resolved
tests/unit/conftest.py Outdated Show resolved Hide resolved
bencekov added a commit that referenced this pull request Mar 22, 2023
@bencekov bencekov dismissed stale reviews from nsklikas and natalian98 via d600201 March 22, 2023 19:48
gruyaume
gruyaume previously approved these changes Mar 22, 2023
Testing if I can push commits to canonical repository branches.
@bencekov bencekov merged commit e3cd966 into main Mar 23, 2023
@bencekov bencekov deleted the IAM-160-Create-charmed-operator branch March 23, 2023 15:06
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.

None yet

4 participants