Skip to content

Conversation

@alexanderadam
Copy link
Contributor

refs #267

PS: I'm looking for a new adventure in case anybody is looking to hire or work with a Ruby/Rails/Crystal dev
PPS: would you be so kind and add the hacktoberfest-accepted label to this issue in case you find that PR helpful? 🥺

Copy link
Contributor

@nfgrep nfgrep left a comment

Choose a reason for hiding this comment

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

Either make the helper generic enough to apply to any fpath or move the logic into file_state_repository and have it be specific to session fpaths

Copy link
Contributor

Thanks for sending this! I'd recommend the latter suggestion from @nfgrep (move it into the file_state_repository).

I think general filename safety is important, but there are enough different ways that a file name might be generated that I don't want to settle on one global answer to soon. Until we find ourselves implementing several similar instances of a safe filename utility for a different specific purposes, it's a lot easier to keep things like this purpose-specific

@alexanderadam alexanderadam force-pushed the fix/avoid_too_long_file_names branch from c1b833d to bfca361 Compare October 22, 2025 14:07
@alexanderadam
Copy link
Contributor Author

I moved the logic into lib/roast/workflow/file_state_repository.rb now and the file suffix is now an optional argument defaulting to .json. 🙂

Copy link
Contributor

@nfgrep nfgrep left a comment

Choose a reason for hiding this comment

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

Appreciate the move into file_state_repository.rb ❤️ Just a few straggler questions to answer, with those addressed we should be good to merge :)

@alexanderadam alexanderadam force-pushed the fix/avoid_too_long_file_names branch from bfca361 to 83239dc Compare October 23, 2025 22:41
@alexanderadam alexanderadam force-pushed the fix/avoid_too_long_file_names branch from 83239dc to 3824daa Compare October 23, 2025 22:48
@alexanderadam alexanderadam requested a review from nfgrep October 24, 2025 14:17
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.

3 participants