Skip to content

Conversation

@sfc-gh-lwilby
Copy link
Collaborator

@sfc-gh-lwilby sfc-gh-lwilby commented Mar 6, 2025

Describe your changes

In this PR, we added a new def code function in code.py which replaces the one in markdown.py.

Since CodeMixin comes first in the definition of DeltaGenerator before MarkdownMixin, the CodeMixin version of the function takes precedence due to python's MRO algorithm and we can safely remove this function.

GitHub Issue Link (if applicable)

Testing Plan

Covered by existing tests.


Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@sfc-gh-lwilby sfc-gh-lwilby changed the title Remove Deprecated Code Function [WIP] Remove Deprecated Code Function Mar 6, 2025
@sfc-gh-lwilby sfc-gh-lwilby added security-assessment-completed Security assessment has been completed for PR impact:internal PR changes only affect internal code change:refactor PR contains code refactoring without behavior change labels Mar 6, 2025
@sfc-gh-lwilby sfc-gh-lwilby changed the title [WIP] Remove Deprecated Code Function Remove Deprecated Code Function Mar 7, 2025
Copy link
Collaborator

@lukasmasuch lukasmasuch left a comment

Choose a reason for hiding this comment

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

LGTM 👍 good catch! thats not good that we have dead code :( wondering if there is a linting rule that could have prevented that. Maybe something to add to tech debt

@sfc-gh-lwilby
Copy link
Collaborator Author

sfc-gh-lwilby commented Mar 7, 2025

LGTM 👍 good catch! thats not good that we have dead code :( wondering if there is a linting rule that could have prevented that. Maybe something to add to tech debt

Hmmm, I think it would need to be a custom rule for Streamlit that checks the delta generator mixins for duplicate functions. We could use that library Bob was using for code generation to add some custom linting rules, I have used it for that in the past.

@sfc-gh-lwilby sfc-gh-lwilby merged commit 17e5c31 into develop Mar 7, 2025
33 checks passed
@sfc-gh-lwilby sfc-gh-lwilby deleted the refactor/remove-deprecated-code-function branch March 7, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:refactor PR contains code refactoring without behavior change impact:internal PR changes only affect internal code security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants