-
Notifications
You must be signed in to change notification settings - Fork 63
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
Add pretty format for command 'get_token' #195
Conversation
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.
@m-sameer can you try to figure out a more generalized method to pretty print CLI outputs? Like in this case we are hardcoding the color of the token can we do it in a more generalized manner
I would suggest that we build a colour palette with a string having a certain colour, a link having a certain colour, names of challenges, date, username etc. What about that? |
@m-sameer Yes, that's right. We all can have a discussion and decide a color. |
Yes. Links can be blue. The classics way. |
To manage the discussion we can list down commands which need to be designed. Blue links and green tokens would suite best. Also, as @Ram81 added that we would need a more generalized manner and not hardcoded colors. To understand this context please have a look at challenge table design in which a single function is used to summarize all the challenge related CLI commands. |
So what do I do? Have some global variables (of colours) for different information? |
@pushkalkatara How should I proceed further? |
Yes, as in the challenge.py there is single function for all the challenge related commands. You can create for other files and commands as well. I have extended the time on GCI dashboard. |
Where do I store the variables? |
f1b9d46
to
25e75d3
Compare
@pushkalkatara Please review. |
@m-sameer Do we need a class to store variables? |
We need a |
Can we think of other ways? Maybe a dictionary? |
Why a dictionary though? |
@pushkalkatara Please review. |
Hi @m-sameer Sorry for the late response. Here's the expectation from this issue:
Actually I just found that there is some replication in the code in |
@pushkalkatara What do I do in that function? Copy paste this function? |
@pushkalkatara please reply |
11f03f2
to
2f1f158
Compare
For now, it looks good and we can approve the GCI task. I think we can more generalize the pretty print commands format by using it for other CLI commands too. One liner function doesn't look good, maybe when we extend it for other CLI commands, it would have a better format. What do you guys think @Ram81 @vkartik97 |
Well, I got screwed here. |
@pushkalkatara Please reply. |
@m-sameer Don't worry. This PR got a bit delayed, you did a good job but the expectations changed with time (like we discovered a new issue that there is code replication, also we discovered that we can more generalize the pretty-print function). I apologize if I was late with the response. I am approving your GCI task as you have solved the issue and helped us to discover more issues. We'll keep the PR on hold for a bit until we resolve the replication of get_token command. As this PR and GCI task has several instances, and you are also familiar with the code, you can retake another pretty print functionality if you're interested. |
@pushkalkatara Well I am having my feet cut off because of the extreme delay and that takes my 30 days from the contest. I can never ever win this contest because of this single PR. |
@m-sameer Hey don't worry you still have a lot of time and there are plenty of tasks. Keep your spirits up and make most learning out of GCI. |
Fix #181: Add pretty format for command 'get_token'
Before:
After: