Skip to content

Add newline before welcome message#11463

Merged
QilongTang merged 1 commit intomasterfrom
ConsoleNewLine
Feb 8, 2021
Merged

Add newline before welcome message#11463
QilongTang merged 1 commit intomasterfrom
ConsoleNewLine

Conversation

@QilongTang
Copy link
Contributor

@QilongTang QilongTang commented Feb 8, 2021 •
edited
Loading

Please Note:

  1. Before submitting the PR, please review How to Contribute to Dynamo
  2. Dynamo Team will meet 1x a month to review PRs found on Github (Issues will be handled separately)
  3. PRs will be reviewed from oldest to newest
  4. If a reviewed PR requires changes by the owner, the owner of the PR has 30 days to respond. If the PR has seen no activity by the next session, it will be either closed by the team or depending on its utility will be taken over by someone on the team
  5. PRs should use either Dynamo's default PR template or one of these other template options in order to be considered for review.
  6. PRs that do not have one of the Dynamo PR templates completely filled out with all declarations satisfied will not be reviewed by the Dynamo team.
  7. PRs made to the DynamoRevit repo will need to be cherry-picked into all the DynamoRevit Release branches that Dynamo supports. Contributors will be responsible for cherry-picking their reviewed commits to the other branches after a LGTM label is added to the PR.

Purpose

Per https://jira.autodesk.com/browse/DYN-3395, Add new line break before displaying welcome message
image

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated

Reviewers

@DynamoDS/dynamo

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@QilongTang QilongTang requested a review from mmisol February 8, 2021 15:29
@mmisol
Copy link
Collaborator

mmisol commented Feb 8, 2021

@QilongTang I think the line break helps although I don't think it's doing exactly what was required, which was to separate first time load logs from workspace logs. Clearing workspace appears to be the first log related to it. Maybe let's ask @Amoursol if he thinks this is ok as it is.

@Amoursol
Copy link
Contributor

Amoursol commented Feb 8, 2021

@QilongTang I think the line break helps although I don't think it's doing exactly what was required, which was to separate first time load logs from workspace logs. Clearing workspace appears to be the first log related to it. Maybe let's ask @Amoursol if he thinks this is ok as it is.

This separates start-up with loading I think. As a user, I can safely ignore the stuff above the line break and may be curious about the stuff below it 😊 I think it's good to merge as is.

A better test would be to include a lot of Extensions to see the change as a user would:

  • Iris
  • Monocle
  • DynaMonito
  • Designtech
  • DynaMaps
  • Data-Shapes

@QilongTang
Copy link
Contributor Author

QilongTang commented Feb 8, 2021 •
edited
Loading

@mmisol @Amoursol Here is what it looks like with some popular package installed. Let me know.
image

@Amoursol
Copy link
Contributor

Amoursol commented Feb 8, 2021

@mmisol @Amoursol Here is what it looks like with some popular package installed. Let me know.
image

LGTM on the improvement @QilongTang

mmisol
mmisol approved these changes Feb 8, 2021
View reviewed changes
Copy link
Collaborator

@mmisol mmisol left a comment

Choose a reason for hiding this comment

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

From a code perspective I see no issues 😄

Approving in place of @Amoursol

@QilongTang
Copy link
Contributor Author

QilongTang commented Feb 8, 2021

@mmisol Thanks. Merging

@QilongTang QilongTang merged commit b09d895 into master Feb 8, 2021
@QilongTang QilongTang deleted the ConsoleNewLine branch February 8, 2021 17:20
@QilongTang QilongTang mentioned this pull request Feb 11, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@mmisol mmisol mmisol approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@QilongTang @mmisol @Amoursol