search

hpcc-systems / DataPatterns

HPCC Systems ECL bundle that provides some basic data profiling and research tools to an ECL programmer
3 stars 4 forks source link

feat: Add Report section to README.md #36

Closed jbrundage closed 5 years ago

jbrundage commented 5 years ago

Signed-off-by: Jaman Brundage jbrundage372@gmail.com

jbrundage commented 5 years ago

@dcamper @GordonSmith could you guys review?

jbrundage commented 5 years ago

@GordonSmith @dcamper I've amended this PR with your requested changes. I've add the two images to the PR, but it's common practice to use https://user-images.githubusercontent.com for README image URLs.

dcamper commented 5 years ago

@GordonSmith @jbrundage Regarding embedded image location: I did some digging and found conflicting information, some of that due to age (GitHub has changed their recommendations over time). However, take a look at https://stackoverflow.com/questions/10189356/how-to-add-screenshot-to-readmes-in-github-repository. The first answer talks about using a branch to store images, along using ?raw=true in the URL.

I don't really want to put binary images in the bundle for every user to download. Thoughts?

jbrundage commented 5 years ago

That stackoverflow thread was where I found the suggested use of https://user-images.githubusercontent.com, but another comment on that thread links to Github's "about-readmes" article where they recommend "using relative links to refer to other files within your repository" here: https://help.github.com/en/articles/about-readmes#relative-links-and-image-paths-in-readme-files

The two files are only ~50kb each, and might catch the eye of someone who clones the repository without noticing the Report section of the README.md

dcamper commented 5 years ago

If someone uses the ecl binary to install DataPatterns as a bundle, the images would come along as well. They would pretty much be a waste of space for them.

jbrundage commented 5 years ago

Then maybe we should just use the generated links. I'm sure they'll work just fine for x-number of years (Where x is the number of years until some cataclysmic event brings down the internet as we know it).

dcamper commented 5 years ago

I'm willing to give it a try. If, after merging with the master branch, the links break then we'll figure out how to deal with them.

jbrundage commented 5 years ago

I've amended this PR to remove the two image files.

dcamper commented 5 years ago

One last change, please: Move the entire report section so that it precedes the "Profile From Path" section, and rename it to "Summary Report with Graphs".

That should do it, I think.

jbrundage commented 5 years ago

Done