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

Added modules for save function #43

Merged
merged 21 commits into from
Jul 27, 2022
Merged

Added modules for save function #43

merged 21 commits into from
Jul 27, 2022

Conversation

KarthikL1729
Copy link
Collaborator

Added save.py with modules for save and reload functions.
Added parse support in main.py for save and reload flags.
Modified filename to be optional argument to accommodate reload flag.
Modified install script to make a directory in ~/.config for sootty to save files.

@KarthikL1729 KarthikL1729 changed the title Feature/save flag Added modules for save function Jun 22, 2022
@KarthikL1729 KarthikL1729 requested a review from Ben1152000 June 30, 2022 15:18
@yihuajack
Copy link
Collaborator

Added a small fix in INSTALL to allow mkdir to make parent directories as needed.

Copy link
Collaborator

@yihuajack yihuajack left a comment

Choose a reason for hiding this comment

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

Functionality correct

@KarthikL1729
Copy link
Collaborator Author

Added fix for issue #44

@KarthikL1729 KarthikL1729 added documentation Improvements or additions to documentation enhancement New feature or request query language Related to improving the query language labels Jul 11, 2022
Copy link
Owner

@Ben1152000 Ben1152000 left a comment

Choose a reason for hiding this comment

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

It looks pretty good, although there are a couple of implementation details that should be modified. We can discuss tomorrow if you have any questions about the feedback.

@KarthikL1729 KarthikL1729 requested a review from Ben1152000 July 14, 2022 09:47
@KarthikL1729
Copy link
Collaborator Author

Just solved all the issues, can this be merged before working on the YAML implementation? Or does it need any further changes?

Copy link
Owner

@Ben1152000 Ben1152000 left a comment

Choose a reason for hiding this comment

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

I tested out the save/reload feature and I found a few issues with how errors/bad input is handled. It would be great if you could fix these issues before merging.

@Ben1152000
Copy link
Owner

Ben1152000 commented Jul 17, 2022

Just solved all the issues, can this be merged before working on the YAML implementation? Or does it need any further changes?

We can worry about that in a future PR. I'll just add an issue for now.

@KarthikL1729
Copy link
Collaborator Author

Fixed all mentioned issues.

@KarthikL1729
Copy link
Collaborator Author

I'm merging the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request query language Related to improving the query language
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants