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

Survey command and info, more documentation and bugfixes #2174

Merged
merged 16 commits into from
Jul 26, 2022
Merged

Survey command and info, more documentation and bugfixes #2174

merged 16 commits into from
Jul 26, 2022

Conversation

JerBouma
Copy link
Contributor

@JerBouma JerBouma commented Jul 25, 2022

This PR does the following:

  • Add a command survey that opens the most recent survey.
  • Add a link within the documentation to fill in the most recent survey.
  • Add a link in the exit message about filling in the survey.
  • For the bugfix for about, I noticed that within the ta menu typing about doesn't work since it adds in "terminal" twice. This was a very minor fix from changing an if-statement to elif.
  • Also update documentation for settings, featflags and sources which now has a proper section in the Getting Started page.

The link, https://openbb.co/survey, still needs to be defined.

image

image

image

image

@JerBouma JerBouma added the docs Code documentation label Jul 25, 2022
@JerBouma JerBouma changed the title Add a command as well as links to the survey Add a command as well as links to the survey + bugfix for about Jul 25, 2022
@JerBouma JerBouma changed the title Add a command as well as links to the survey + bugfix for about Add a command as well as links to the survey + bugfix for about comand Jul 25, 2022
@JerBouma JerBouma changed the title Add a command as well as links to the survey + bugfix for about comand Add a command as well as links to the survey + bugfix for about command Jul 25, 2022
@minhhoang1023
Copy link
Contributor

Thanks for setting this up Jer! 🙌

@piiq @andrewkenreich A quick question about the installer! My idea is that we can add the Survey link in the exit message for the Terminal. @JerBouma mentioned that it will only appear when people type q to exit. Is there any way we can still display the message when people exit in a different way?

@piiq
Copy link
Contributor

piiq commented Jul 25, 2022

Is there any way we can still display the message when people exit in a different way?

If the user closes the window the window just closes and there is no exit per se. We can display survey link in the start message, or in some other place

@JerBouma JerBouma changed the title Add a command as well as links to the survey + bugfix for about command Add commands and info about the survey, bugfix about and get some more documentation in Jul 25, 2022
@JerBouma JerBouma marked this pull request as ready for review July 25, 2022 21:14
@JerBouma JerBouma changed the title Add commands and info about the survey, bugfix about and get some more documentation in Survey command and info, more documentation and bugfixes Jul 25, 2022
@minhhoang1023
Copy link
Contributor

It throws an error when I launched the command.

image

terminal.py Outdated Show resolved Hide resolved
@JerBouma
Copy link
Contributor Author

Fixed the survey command not working. Does it matter much it isn't a static method?

@minhhoang1023
Copy link
Contributor

@JerBouma it's software engineering best practice. So that an instance doesn't have to be instantiated each time you call it.

It would take less than 1 min to fix it, and we can keep the code base to a high-quality standard.

@minhhoang1023
Copy link
Contributor

Awesome! Working now 🚀

@JerBouma
Copy link
Contributor Author

JerBouma commented Jul 26, 2022

@JerBouma it's software engineering best practice. So that an instance doesn't have to be instantiated each time you call >it.

It would take less than 1 min to fix it, and we can keep the code base to a high-quality standard.

Could you suggest how?

@minhhoang1023
Copy link
Contributor

For the survey, we could add "Take 2 mins to fill in our survey" or "Fill in our 2-minute survey". Letting people know that this is extremely QUICK to do!

Looks great overall

@JerBouma
Copy link
Contributor Author

image

image

Better?

@minhhoang1023
Copy link
Contributor

@JerBouma it's software engineering best practice. So that an instance doesn't have to be instantiated each time you call >it.
It would take less than 1 min to fix it, and we can keep the code base to a high-quality standard.

Could you suggest how?

Just need to add the @staticmethod and remove the self. Like this

    @staticmethod
    def call_survey(_) -> None:
        """Process survey command"""
        webbrowser.open("https://openbb.co/survey")

@JerBouma
Copy link
Contributor Author

Done done

Copy link
Contributor

@minhhoang1023 minhhoang1023 left a comment

Choose a reason for hiding this comment

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

LGG

@piiq piiq merged commit f3a6a11 into OpenBB-finance:main Jul 26, 2022
@JerBouma JerBouma deleted the link_promotions branch July 26, 2022 10:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Code documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants