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

feat(irc): Initial charm #3

Merged
merged 7 commits into from
Jul 17, 2024
Merged

feat(irc): Initial charm #3

merged 7 commits into from
Jul 17, 2024

Conversation

merkata
Copy link
Collaborator

@merkata merkata commented Jun 17, 2024

Adds the bare bones of the charm, so snap operations. Needs handling of relations to PSql and Matrix/Synapse for generation of the config files (will be done via any-charm in the integration tests), needs tests.

@merkata merkata requested a review from a team as a code owner June 17, 2024 09:15
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

license-eye has checked 38 files.

Valid Invalid Ignored Fixed
0 1 37 0
Click to see the invalid file list
  • lib/charms/operator_libs_linux/v2/snap.py
Use this command to fix any missing license headers
```bash

docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix

</details>

@@ -0,0 +1,1160 @@
# Copyright 2021 Canonical Ltd.
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
# Copyright 2021 Canonical Ltd.
# Copyright 2024 Canonical Ltd.
# Licensed under the Apache2.0. See LICENSE file in charm source for details.
# Copyright 2021 Canonical Ltd.

yanksyoon
yanksyoon previously approved these changes Jun 17, 2024
Copy link

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

Nice snap package! LGTM!

src/irc.py Outdated Show resolved Hide resolved
src/irc.py Outdated Show resolved Hide resolved
src/irc.py Outdated Show resolved Hide resolved
src/charm.py Outdated
# Copyright 2024 Canonical Ltd.
# See LICENSE file for licensing details.

"""Charm for bind."""

Choose a reason for hiding this comment

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

Minor: bind -> irc-bridge-operator

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You can treat it as an oversight on my end or as an unintentional (though well deserved) acknowledgement that I took the great work in the bind-operator and adjusted it here. Let it be written in (git) history!

amandahla
amandahla previously approved these changes Jun 17, 2024
Copy link

@amandahla amandahla left a comment

Choose a reason for hiding this comment

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

Just need some replacements here and there
"bind" to "irc-bridge" :)

Copy link

@yanksyoon yanksyoon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes!

Choose a reason for hiding this comment

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

Out of curiosity, would this be used later? (or can we delete it?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Might be used and it is part of the template repo, so that's why I'm keeping it. (I added the templates as I messed up the terraform config for this repo and they were not added initially...).

@merkata merkata merged commit 2f36351 into main Jul 17, 2024
13 of 17 checks passed
@merkata merkata deleted the feat/initial-charm branch July 17, 2024 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants