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

refactor(ia): import PyStringMap from org.python.core #262

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

cesarcoatl
Copy link
Member

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our commit message format
  • All applicable pre-commit hooks have passed when running pre-commit run --all-files
  • All tox tests have succeeded

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

We import PyStringMap from copy in both src.com.inductiveautomation.ignition.common.script and use Python 3.11 for running CI.

Issue Number: N/A

What is the new behavior?

We import from org.python.core and use coatl-dev/workflows@v3 and Python 3.12 for CI.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

use coatl-dev/workflows@v3
replace Python 3.11 with 3.12

use coatl-dev/workflows@v3
replace Python 3.11 with 3.12
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

PR Type: Refactoring

PR Summary: This pull request encompasses a series of updates aimed at refining the project's workflow configurations and codebase. Specifically, it updates various GitHub Actions workflows to use a newer version, transitions CI to utilize Python 3.12, and adjusts the source of the PyStringMap import within the project's Python code. These changes are intended to enhance the project's compatibility with newer technologies and improve code maintainability.

Decision: Comment

📝 Type: 'Refactoring' - not supported yet.
  • Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
📝 Complexity: the changes are too large or complex for Sourcery to approve.
  • Unsupported files: the diff contains files that Sourcery does not currently support during reviews.

General suggestions:

  • Given the concerns raised about maintaining support for Python 2.7, especially in the context of dependency management and security, it may be beneficial to reassess the necessity of this support. Considering Python 2.7's end-of-life status, focusing on compatibility with more current versions of Python could be more advantageous for the project's future development.
  • The addition of PyStringMap from org.python.core suggests an effort to align more closely with Jython's native libraries. It's important to ensure that this change does not introduce any regressions or compatibility issues with existing scripts and functionalities.
  • The use of a pylint directive to disable a warning about PyStringMap not being recognized suggests there might be an underlying issue with the environment setup or import paths. It would be prudent to investigate this further to ensure that it does not mask any potential problems that could affect code quality or maintainability.

Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨

Share Sourcery

Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

@cesarcoatl cesarcoatl merged commit 824318f into main Feb 22, 2024
3 checks passed
@cesarcoatl cesarcoatl deleted the refactor/PyStringMap branch February 22, 2024 00:28
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.

1 participant