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

Remove simulate intro to align with good CLI practices #1427

Merged
merged 4 commits into from May 17, 2023

Conversation

J535D165
Copy link
Member

@J535D165 J535D165 commented May 6, 2023

No description provided.

@J535D165
Copy link
Member Author

J535D165 commented May 6, 2023

Remove


            _____ _____            _
     /\    / ____|  __ \          (_)
    /  \  | (___ | |__) |_____   ___  _____      __
   / /\ \  \___ \|  _  // _ \ \ / / |/ _ \ \ /\ / /
  / ____ \ ____) | | \ \  __/\ V /| |  __/\ V  V /
 /_/    \_\_____/|_|  \_\___| \_/ |_|\___| \_/\_/

---------------------------------------------------------------------------------
|                                                                                |
|  Welcome to ASReview LAB - AI-assisted systematic reviews software.            |
|  In simulation mode the computer will simulate how well ASReview LAB           |
|  could have accelerate the systematic review of your dataset.                  |
|  You can sit back and relax while the computer runs this simulation.           |
|                                                                                |
|  GitHub page:        https://github.com/asreview/asreview                      |
|  Questions/remarks:  asreview@uu.nl                                            |
|                                                                                |
---------------------------------------------------------------------------------


@J535D165 J535D165 mentioned this pull request May 16, 2023
@jteijema
Copy link
Member

I support this removal, but that doesn't say I won't miss the intro.

@jspaaks
Copy link

jspaaks commented May 17, 2023

Out of interest, why was this considered a bad practice?

@J535D165
Copy link
Member Author

It's opinionated, of course :) In large-scale simulation studies, this is overwhelming. Some guidelines I think that might apply:

Display output on success, but keep it brief. Traditionally, when nothing is wrong, UNIX commands display no output to the user. This makes sense when they’re being used in scripts, but can make commands appear to be hanging or broken when used by humans. For example, cp will not print anything, even if it takes a long time. (https://clig.dev/#output)

Reducing cognitive load is necessary for making an accessible product. (https://primer.style/cli/)

Also, help should be at the --help when I interpret these guides.

I see this change as the start of implementing a more informative interface (with progress bars for more model components as mentioned in other PRs).

@PeterLombaers
Copy link
Member

Looks good to me. It does mean less scrolling when you are running multiple simulations and you are scanning the output.

@PeterLombaers PeterLombaers merged commit 41623f9 into master May 17, 2023
17 checks passed
@J535D165 J535D165 deleted the remove-simulate-intro branch May 17, 2023 13:20
@jspaaks
Copy link

jspaaks commented May 17, 2023

Thanks for the explanation and links!

cskaandorp pushed a commit to cskaandorp/asreview that referenced this pull request Apr 22, 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.

None yet

4 participants