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

Back-end code cleanup, docs, style #91

Merged
merged 15 commits into from
Aug 20, 2024
Merged

Back-end code cleanup, docs, style #91

merged 15 commits into from
Aug 20, 2024

Conversation

reelmatt
Copy link
Member

@reelmatt reelmatt commented May 9, 2020

All these changes should mostly deal with style/documentation and not affect any functionality. There are three which have the potential to (if I missed a reference in one place or the other), so might need extra care during review, which are:

  1. Change mentions of react in the saved workflow to ui-graph. Addresses code review feedback in case the front-end changes to a non-React framework.
  2. Changes the Workflow to_session_dict method to a to_json method. Both return dict-like objects so this should keep existing functionality. It adds the node_dir attribute which had been missing.
  3. Change node update endpoint from POST -> PATCH to make it more RESTful (addresses code review feedback). Front-end has been updated as well.

Additional minor changes, are all clean-up/style:

  • Uses flow_vars for Node options instead of the old **self.options approach.
  • Remove unused import statements
  • Add/update docstrings for Node methods
  • Remove completed TODO comments
  • workflow.py lists a lot of changes, but this is all re-ordering the existing methods to make the layout cleaner. I.e., "node" methods are grouped together, "getters/setters", etc. Now not just a hodgepodge of methods that can be hard to locate easily.

@matthew-t-smith matthew-t-smith merged commit af02519 into master Aug 20, 2024
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.

3 participants