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 examples/ folder from stan analysis #98

Merged

Conversation

german1608
Copy link
Contributor

Not worth an issue I'd say, let me know otherwise :)

While working on #97 I noticed this:

Screen Shot 2022-10-23 at 3 21 52 PM

Additional tasks

  • Documentation for changes provided/changed
  • Tests added
  • Updated CHANGELOG.md

@chshersh
Copy link
Owner

I would actually better fix Stan warning than ignoring it in examples.

@german1608 german1608 changed the title Remove examples/ folder from stan analysis Update examples/ folder from stan analysis Oct 25, 2022
@chshersh chshersh added the refactoring Improving code quality, reducing tech debt label Jan 8, 2023
@chshersh chshersh added this to the v0.1.0.0: Improved UX milestone Jan 8, 2023
Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Looks nice!

It took me a while to review this small PR but I'm happy to merge it 🙂

Could you rebase it on top of the latest main branch?

@german1608 german1608 force-pushed the exclude-examples-lhs-files-from-stan-report branch from 9ebcb61 to 1fb749d Compare January 13, 2023 17:48
@chshersh
Copy link
Owner

Hey @german1608 👋🏻

There're some non-trivial git conflicts in this PR. I would appreciate if you could find the time to resolve them so I could merge the PR 🙏🏻

If you don't have the capacity, that's also fine, just let me know 🙂

@german1608
Copy link
Contributor Author

@chshersh i will take a look this weekend. Thanks for letting me know 👍

@german1608 german1608 force-pushed the exclude-examples-lhs-files-from-stan-report branch from 1fb749d to be69b45 Compare January 20, 2023 18:15
@@ -181,11 +181,11 @@ app = do
formattedPrinter "Starting grepping 🔥" Colourista.white
file <- liftIO $ TIO.readFile filePath

let fileName = "file name: " `T.append` (last $ T.split (== '/') $ T.pack filePath)
let fileNameMessage = "file name: " `T.append` (last $ T.split (== '/') $ T.pack filePath)
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm, the last commit messages says that it uses takeFileName to get rid of the last function usage and remove Stan warning but I still see last here 🤔

Maybe something was lost during conflict resolution?

@chshersh
Copy link
Owner

Hey @german1608 👋🏻

I have one last question about this PR. I would appreciate if you could find time to look into it 🙂

@german1608
Copy link
Contributor Author

Hi @chshersh

Sorry for the delay, had a lot of errands and events to attend. Taking a look at this

@german1608 german1608 force-pushed the exclude-examples-lhs-files-from-stan-report branch from be69b45 to 8a1fe33 Compare January 30, 2023 12:09
Copy link
Owner

@chshersh chshersh left a comment

Choose a reason for hiding this comment

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

Thanks a ton! 🙏🏻

@chshersh chshersh merged commit d076bb0 into chshersh:main Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Improving code quality, reducing tech debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants