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

Security/README.rst: Fix template rendering #13

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

KVGarg
Copy link

@KVGarg KVGarg commented Nov 18, 2018

There was as issue with GitHub rst generator. The issue
has been solved by GitHub. Making a small change in file,
fixes the template rendering problem.

Fixes #10

@KVGarg
Copy link
Author

KVGarg commented Nov 18, 2018

Hey @adtac @li-boxuan , can you review out this PR ?

Copy link
Member

@li-boxuan li-boxuan left a comment

Choose a reason for hiding this comment

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

Thanks for finding a workaround. Apparently, it is rendered now. What change did you do to the file?
Files changed tab is saying that Binary file not shown. Maybe the file is wrongly encoded. Is it possible to change it to a normal file?

@KVGarg
Copy link
Author

KVGarg commented Nov 23, 2018

I actually just added more = below the title How to Fix this.
Changing to normal file means converting it to .md file ? or revert back to original file?

Copy link
Member

@li-boxuan li-boxuan left a comment

Choose a reason for hiding this comment

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

GitHub says it is a Binary file, so the diff is not shown. Can you try removing the file, then create a new rst file and paste the content? And see if it becomes a regular file. Or see if there is any other solution.

@KVGarg
Copy link
Author

KVGarg commented Dec 4, 2018

Ok, I will try to find a solution so that difference is visible

@KVGarg KVGarg force-pushed the master branch 6 times, most recently from fdb3635 to a3d38ab Compare December 4, 2018 13:14
@KVGarg
Copy link
Author

KVGarg commented Dec 4, 2018

I tried to delete the previous file and paste the content of previous file to a newly created file, but then also GitHub was showing that message "Binary file not shown". And when I pushed a new file without deleting the old file then it is clearly displaying the difference. Also, Renaming the file doesn't seems to fix the problem.
I think there is still some problem with GitHub cache that's why it is displaying that message `Binary file not shown".
Possible solution that I'm able to figure out is -

  1. For now push these two files together and after getting PR merged create a new PR which will delete old file and rename new file to README.rst or
  2. divide this issue into two commits - one for deleting the folder Security and in other commit re-add the folder with newly created file README.rst

I have checked out the last solution and it is working perfectly fine, the only thing is that it is divided into 2 commits.
Also, for solution no.1, I have tried to push a file with same content as that of README.rst and it was clearly showing the difference with existence of previous file also.
So both the solution are working

@KVGarg
Copy link
Author

KVGarg commented Dec 11, 2018

@li-boxuan can you help me out, what to do for now ?

Copy link
Member

@utkarsh2102 utkarsh2102 left a comment

Choose a reason for hiding this comment

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

Why is there a binary file?
Also, you could have written a good commit message.

@KVGarg
Copy link
Author

KVGarg commented Dec 17, 2018

That binary file is the original README.rst file, that GitHub is recognizing it as a binary file. That's why the any changes being made in it, aren't being displayed. To solve this issue, I found two ways that I have suggested above. We can go with any of them. The only thing is it will take 2 commits. I have implemented the first way that's why we are able to a new file README2.rst

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

https://github.com/coala/aspect-docs/blob/master/Root/Security/README.rst broken
3 participants