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

Add a formatted representation of the UpdatePageAction dataclass #230

Merged
merged 7 commits into from
Feb 2, 2024

Conversation

mthaddon
Copy link

Applicable spec: N/A

Overview

Add a formatted representation of the UpdatePageAction dataclass.

Rationale

This relates to #226. I've started by only adding for one dataclass to get some feedback on the approach before I apply it more widely.

Module Changes

N/A

Checklist

  • The contributing guide was applied
  • The documentation is generated using src-docs
  • The PR is tagged with appropriate label (urgent, trivial, complex)
  • Changelog has been updated
  • Version has been incremented

Haven't incremented the version as I'm not sure if that's appropriate.

@mthaddon mthaddon requested a review from a team as a code owner January 17, 2024 16:46
Copy link

@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 totally checked 98 files.

Valid Invalid Ignored Fixed
0 60 38 0
Click to see the invalid file list
  • Dockerfile
  • action.yaml
  • generate-src-docs.sh
  • main.py
  • src/init.py
  • src/action.py
  • src/check.py
  • src/clients.py
  • src/commit.py
  • src/constants.py
  • src/content.py
  • src/discourse.py
  • src/docs_directory.py
  • src/download.py
  • src/exceptions.py
  • src/index.py
  • src/metadata.py
  • src/migration.py
  • src/navigation_table.py
  • src/reconcile.py
  • src/repository.py
  • src/sort.py
  • src/types_.py
  • tests/init.py
  • tests/conftest.py
  • tests/factories.py
  • tests/integration/init.py
  • tests/integration/conftest.py
  • tests/integration/test___init__run_conflict.py
  • tests/integration/test___init__run_migrate.py
  • tests/integration/test___init__run_reconcile.py
  • tests/integration/test_discourse.py
  • tests/integration/types.py
  • tests/types.py
  • tests/unit/init.py
  • tests/unit/action/init.py
  • tests/unit/action/test_other_actions.py
  • tests/unit/action/test_update_action.py
  • tests/unit/conftest.py
  • tests/unit/helpers.py
  • tests/unit/test___init__.py
  • tests/unit/test_check.py
  • tests/unit/test_commit.py
  • tests/unit/test_content.py
  • tests/unit/test_discourse.py
  • tests/unit/test_docs_directory.py
  • tests/unit/test_download.py
  • tests/unit/test_index.py
  • tests/unit/test_index_contents_get.py
  • tests/unit/test_index_contents_hierarchy.py
  • tests/unit/test_index_contents_parse.py
  • tests/unit/test_metadata.py
  • tests/unit/test_migration/init.py
  • tests/unit/test_migration/test_private.py
  • tests/unit/test_migration/test_public.py
  • tests/unit/test_navigation_table.py
  • tests/unit/test_reconcile.py
  • tests/unit/test_repository.py
  • tests/unit/test_sort.py
  • tests/unit/test_types_.py

jdkandersson
jdkandersson previously approved these changes Jan 18, 2024
Copy link
Contributor

@jdkandersson jdkandersson left a comment

Choose a reason for hiding this comment

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

Looks fine, please update the changelog. A lot of the issues with the checks should be resolved in #231

Copy link

@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 totally checked 98 files.

Valid Invalid Ignored Fixed
0 60 38 0
Click to see the invalid file list
  • Dockerfile
  • action.yaml
  • generate-src-docs.sh
  • main.py
  • src/init.py
  • src/action.py
  • src/check.py
  • src/clients.py
  • src/commit.py
  • src/constants.py
  • src/content.py
  • src/discourse.py
  • src/docs_directory.py
  • src/download.py
  • src/exceptions.py
  • src/index.py
  • src/metadata.py
  • src/migration.py
  • src/navigation_table.py
  • src/reconcile.py
  • src/repository.py
  • src/sort.py
  • src/types_.py
  • tests/init.py
  • tests/conftest.py
  • tests/factories.py
  • tests/integration/init.py
  • tests/integration/conftest.py
  • tests/integration/test___init__run_conflict.py
  • tests/integration/test___init__run_migrate.py
  • tests/integration/test___init__run_reconcile.py
  • tests/integration/test_discourse.py
  • tests/integration/types.py
  • tests/types.py
  • tests/unit/init.py
  • tests/unit/action/init.py
  • tests/unit/action/test_other_actions.py
  • tests/unit/action/test_update_action.py
  • tests/unit/conftest.py
  • tests/unit/helpers.py
  • tests/unit/test___init__.py
  • tests/unit/test_check.py
  • tests/unit/test_commit.py
  • tests/unit/test_content.py
  • tests/unit/test_discourse.py
  • tests/unit/test_docs_directory.py
  • tests/unit/test_download.py
  • tests/unit/test_index.py
  • tests/unit/test_index_contents_get.py
  • tests/unit/test_index_contents_hierarchy.py
  • tests/unit/test_index_contents_parse.py
  • tests/unit/test_metadata.py
  • tests/unit/test_migration/init.py
  • tests/unit/test_migration/test_private.py
  • tests/unit/test_migration/test_public.py
  • tests/unit/test_navigation_table.py
  • tests/unit/test_reconcile.py
  • tests/unit/test_repository.py
  • tests/unit/test_sort.py
  • tests/unit/test_types_.py

cbartz
cbartz previously approved these changes Jan 18, 2024
jdkandersson
jdkandersson previously approved these changes Jan 19, 2024
@mthaddon mthaddon dismissed stale reviews from jdkandersson and cbartz via 90a9a5a January 19, 2024 13:32
src/types_.py Outdated Show resolved Hide resolved
arturo-seijas
arturo-seijas previously approved these changes Jan 19, 2024
@jdkandersson
Copy link
Contributor

I think main can be merged in now which should address some of the failing checks

@mthaddon
Copy link
Author

I think main can be merged in now which should address some of the failing checks

$ git merge main
Already up to date.

I'll rerun the checks.

@jdkandersson
Copy link
Contributor

The issues are:

  • Black formatting doesn't seem to be applied
  • The integration tests look for the name of the action in the log messages which used to be UpdateAction(...) or similar. Perhaps in the string representation include the action being taken? You can check the integration test for more details, please let me know if you need more help. It is case sensitive at the moment

Copy link

Test coverage for e99b377

Name                      Stmts   Miss Branch BrPart  Cover   Missing
---------------------------------------------------------------------
src/__init__.py              97      0     42      0   100%
src/action.py               155      0     46      0   100%
src/check.py                 67      0     27      0   100%
src/clients.py               12      0      0      0   100%
src/commit.py                42      0     12      0   100%
src/constants.py             10      0      0      0   100%
src/content.py               50      0     10      0   100%
src/discourse.py            159      0     34      0   100%
src/docs_directory.py        33      0      8      0   100%
src/download.py              23      0      2      0   100%
src/exceptions.py            15      0      0      0   100%
src/index.py                142      0     56      0   100%
src/metadata.py              28      0     12      0   100%
src/migration.py            102      0     33      0   100%
src/navigation_table.py      65      0     20      0   100%
src/reconcile.py            123      0     60      0   100%
src/repository.py           284      0     88      0   100%
src/sort.py                  43      0     26      0   100%
src/types_.py               201      0     54      0   100%
---------------------------------------------------------------------
TOTAL                      1651      0    530      0   100%

Static code analysis report

Working... ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ 100% 0:00:01
Run started:2024-01-25 04:42:41.052487

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 17621
  Total lines skipped (#nosec): 3
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@mthaddon mthaddon merged commit 1f56e73 into main Feb 2, 2024
23 checks passed
@mthaddon mthaddon deleted the update-page-action-repr branch February 2, 2024 11:55
javierdelapuente added a commit that referenced this pull request Apr 4, 2024
* Fix nothing to migrate bug (#231)

* handle case where default branch and discourse are in-line but base is not for the migration case

* Add a formatted representation of the UpdatePageAction dataclass (#230)

* Add a formatted representation of the UpdatePageAction dataclass

* Update changelog

* Add return value to docstring

* Include return type

* Apply lint changes

* Include a string representation of the class, which many tests check for

* fix linting (#234)

* Update python Docker tag to v3.12 (#220)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: arturo-seijas <[email protected]>

* Update dependency more-itertools to >=10.2,<10.3 (#229)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: arturo-seijas <[email protected]>

* Update dependency PyGithub to >=2.2,<2.3 (#233)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: arturo-seijas <[email protected]>
Co-authored-by: Christopher Bartz <[email protected]>

* Update dependency pytest to v8 (#232)

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: arturo-seijas <[email protected]>

* Create bot_pr_approval.yaml (#236)

* Removed `discourse-gatekeeper/discourse-ahead-ok` (#235)

* feat(ISD-1567): removed obsolete code and corresponding documentation

* feat(ISD-1498): removed check in test_check for the removed tag

* feat(ISD-1498): updated test parameters to account for removal of discourse ahead tag removal

* feat(ISD-1498): removed tagged argument from tests

* feat(ISD-1498): ensured all parameters return only 2 values in tuple

* feat(ISD-1498): removed ahead tag from constants file, removed mention in integration tests

* feat(ISD-1498): removed ahead tag from check.py

* feat(ISD-1498): rectified erroneous deletion of expected problem in test check

* feat(ISD-1498): adhered to linting requirements by removing unused src.constants imports

* feat(ISD-1498): fixed all linting issues and fixed method signatures.

* feat(ISD-1498): removed test case 7 from test conflicts

* feat(ISD-1498): removed test 7

* feat(ISD-1498): fixed linting issues

* feat(ISD-1498): updated change log with new version details

* feat(ISD-1498): added src-docs

* feat(ISD-1498): fixed comments in run_conflict integration test

* feat(ISD-1498): removed assert associated with removed act

* Prepare discourse-gatekeeper for PAAS app charmer  (#238)


New input parameter `charm-dir`, that defines where to look for `metadata.yaml`, `charmcraft.yaml` or the docs directory.

If `metadata.yaml` does not exist, `charmcraft.yaml` is used instead.

* conflict change race condition resolution stage 1 (#241)

* add check that content has not changed on discourse before updating the content

* New version v0.9.0 (#242)

---------

Co-authored-by: David Andersson <[email protected]>
Co-authored-by: mthaddon <[email protected]>
Co-authored-by: arturo-seijas <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Christopher Bartz <[email protected]>
Co-authored-by: Brendan Bell <[email protected]>
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