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

update snapshots with new align #58

Merged
merged 17 commits into from
Aug 23, 2023
Merged

Conversation

Melkiades
Copy link
Contributor

@Melkiades Melkiades commented Jul 17, 2023

fixes insightsengineering/sme-tasks#57 to be merged only after insightsengineering/rlistings#136

@Melkiades Melkiades added the sme label Jul 17, 2023
DESCRIPTION Outdated Show resolved Hide resolved
Signed-off-by: Davide Garolini <dgarolini@gmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jul 17, 2023

Unit Tests Summary

    1 files  112 suites   3m 18s ⏱️
249 tests 238 ✔️   11 💤 0
501 runs  253 ✔️ 248 💤 0

Results for commit 234ff79.

♻️ This comment has been updated with latest results.

Comment on lines +16 to +19
default_formatting = list(
all = formatters::fmt_config(align = "left"),
numeric = formatters::fmt_config(align = "center")
),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

took this from @BFalquet. If we need to set it up everywhere better to start now ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, is this a default @barnett11? I cannot find the issue where you talked about this, sorry

Choose a reason for hiding this comment

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

This was the discussion, left alignment is needed for this template
https://github.com/insightsengineering/sme-tasks/issues/619

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DESCRIPTION Outdated Show resolved Hide resolved
Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

need to vbump formatters

Co-authored-by: Joe Zhu <sha.joe.zhu@gmail.com>
Signed-off-by: Davide Garolini <dgarolini@gmail.com>
@Melkiades
Copy link
Contributor Author

@shajoezhu could you take a look at this again please?

@shajoezhu
Copy link
Contributor

sorry, I had missed this tag before. @Melkiades , could you please update the code and rerun tests please. thanks!

@shajoezhu shajoezhu self-requested a review August 22, 2023 03:38
@Melkiades
Copy link
Contributor Author

Melkiades commented Aug 22, 2023

sorry, I had missed this tag before. @Melkiades , could you please update the code and rerun tests please. thanks!

ready @shajoezhu! :)

Copy link
Contributor

@shajoezhu shajoezhu left a comment

Choose a reason for hiding this comment

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

lgtm! thanks @Melkiades

@Melkiades Melkiades merged commit 26439a5 into main Aug 23, 2023
23 checks passed
@Melkiades Melkiades deleted the 57_fix_after_align_left@main branch August 23, 2023 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants