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

Add relative_source_path attribute to content #2486

Merged
merged 1 commit into from Jan 27, 2019
Merged

Add relative_source_path attribute to content #2486

merged 1 commit into from Jan 27, 2019

Conversation

bryanbrattlof
Copy link
Contributor

@bryanbrattlof bryanbrattlof commented Dec 1, 2018

I wanted to see what the consensus is for adding a relative_source_path attribute to the Content object.

@egberts, and I in #2484 where talking about using an unpublished way to add a relative path as a comment into the generated html for page and article for better debugging using continuous integration pipelines like gitlab-ci.

I also saw the PR (#2416) wanted the ability to generate urls to edit his content in github, but it seems like it died there.

Hope everyone's weekend it going well.

@avaris
Copy link
Member

avaris commented Dec 2, 2018

This doesn't need to be a property.

@bryanbrattlof
Copy link
Contributor Author

bryanbrattlof commented Dec 2, 2018

Thanks @avaris,

I switched to setting relative_source_path after source_path is set during the creation of the Content object.

I also fixed a typo in the page's attribute table in the documentation that I missed.

Although, I'm having trouble figuring out if this is something others will want vs something that I want. If I'm the only one I can use the content_init signal to attach this for my own use. So feel free to tell me to push off and I'll close this. :)

Thanks again

@avaris
Copy link
Member

avaris commented Dec 2, 2018

For debugging purposes, I see very little benefit in this over source_path but one can use this to have a link to the source. So, I'm not opposed to it.

@bryanbrattlof bryanbrattlof changed the title Add relative_source_path atrtibute to content Add relative_source_path attribute to content Dec 2, 2018
@bryanbrattlof
Copy link
Contributor Author

bryanbrattlof commented Dec 2, 2018

Sorry for the updates, I just now noticed how bad I am at spelling

@egberts
Copy link

egberts commented Dec 2, 2018

Awesome!

Copy link
Contributor

@pedrohdz pedrohdz left a comment

Choose a reason for hiding this comment

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

As much as I would love to see this change in sooner rather than later, there should be a unit test added. tests/test_contents.py seems like the correct place.

@bryanbrattlof
Copy link
Contributor Author

Thanks @pedrohdz for the suggestion. I've added the tests. :)

@justinmayer
Copy link
Member

Many thanks to @bryanbrattlof for the enhancement and to @avaris, @egberts, and @pedrohdz for reviewing. 🎉

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.

None yet

5 participants