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

pm respond to README #3678

Merged
merged 10 commits into from
Jul 26, 2024
Merged

pm respond to README #3678

merged 10 commits into from
Jul 26, 2024

Conversation

ashton22305
Copy link
Contributor

Work on #3377

Will render the README.md or README.txt in a project root to the projects#show page.

@johrstrom
Copy link
Contributor

I'm still looking at this a bit to see if we need all this complexity. It could be the case that we do, but it seems to me that it should be easier.

johrstrom
johrstrom previously approved these changes Jul 25, 2024
Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

Thanks!

@johrstrom
Copy link
Contributor

🤦‍♂️ looks like we have a conflict you need to resolve.

Comment on lines 11 to 14
return sanitize(markdown_html)
elsif File.extname(readme_location) == '.txt'
# simple_format sanitizes its output
return simple_format(file_content)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't going to mention this, but now that I see there's a conflict to resolve I may as well.

You don't need the return statements here. I use solargraph with vscode to see stuff like this. It would tell you you don't need them here. It's really helpful for small idioms like this.

Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

LGTM

@johrstrom johrstrom merged commit 943e81a into master Jul 26, 2024
26 checks passed
@johrstrom johrstrom deleted the project-readme branch July 26, 2024 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants