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

Issue 469 - Fallback to overview.html when pages do not exist for compare option #470

Merged
merged 1 commit into from
Oct 18, 2023

Conversation

rishijain
Copy link
Contributor

Check list:

Description: This fixes issue #469 .

@rishijain rishijain force-pushed the issue469 branch 2 times, most recently from 05b560e to c55ca22 Compare October 16, 2023 13:23
Copy link
Collaborator

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@rishijain Looks good, thanks!

Could you please solve the conflict?

@rishijain rishijain force-pushed the issue469 branch 2 times, most recently from 418c3dc to cc7f5b4 Compare October 17, 2023 17:48
@rishijain
Copy link
Contributor Author

@etagwerker resolved.

root_directory_path = File.expand_path(root_directory)
file_path = "#{root_directory_path}/#{file_name}"
file_path = "#{root_directory_path}/overview.html" unless File.exist?(file_path)
file_path(file_path)
Copy link
Collaborator

@nunosilva800 nunosilva800 Oct 18, 2023

Choose a reason for hiding this comment

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

This is weird. What about naming the local var index_path or just path to not confuse it with the method name file_path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@nunosilva800 oh yes my apologies. How did I not see how bad that naming was. I have used index_path.

@etagwerker etagwerker merged commit 92a20aa into whitesmith:main Oct 18, 2023
26 checks passed
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