-
Notifications
You must be signed in to change notification settings - Fork 7
implemented the (huh???) library #136
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
base: main
Are you sure you want to change the base?
Conversation
|
we can prob resolve conflicts later, this is not much of a priority |
|
Hi Adam, I saw the demo and I think you are doing amazing so far. I will be able to give my review later, currently I am caught up in TA tasks. However, I do have a request for you based on the demo video you sent. Are you able to make it so the huh app doesn't exit immediately after using a command? For example, we can get events, maybe scroll up and down then, and then ask the app to return back to the commands or different menu. The main menu can also have a quit option to close the huh app too. |
|
I think that issue should already be fixed. I'll send another demo this afternoon 🙂 |
|
Demo Part 2: Explanation: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this change with the imports? It's a pointless diff and there is actually a method to the madness rn, we've been doing
import (
standard library packages
<blank line>
third party packages
<blank line>
our packages
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same thing here, this diff doesn't deserve to exist
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really cool and great work but I have suggestions:
- When you're ready to merge, we don't want a file called
huh.gothat you have to run -- the library should integrate cleanly with our existing project structure - This feature may benefit from being broken up into smaller more atomic changes: A really good first feature would be replacing the current input forms in files like
internal/cli/events/put.go
|
Ty for taking a look at it Josh. So, I have the form set at choosing which service to use and test, like choose announcements, officers, or events. Should the starting form be in its own file while the get, put, delete, and post have its own forms in the CLI files already created? Also any preferences on where that starting form should be cuz I cant decide where? |
wait nvm i figured it out now |
Changes:
Additions
let me know if there are any issues :D