-
Notifications
You must be signed in to change notification settings - Fork 6
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
File Tree Diff: initial implementation #424
base: main
Are you sure you want to change the base?
Conversation
Added a new addon to show all the filenames and URLs that were added, deleted or modified in the current PR compared to the LATEST version. Closes #409
OOOF I WANT THIS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like a great start. I'm 👍 on shipping it overall. I'm not well versed on the JS style other than my silly comment below :)
|
||
showDiff() { | ||
// const outdated = objectPath.get(this.config, "addons.filetreediff.oudated", false); | ||
const diffdata = objectPath.get(this.config, "addons.filetreediff.diff"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we probably want camelcase if we're being good JS style:
const diffdata = objectPath.get(this.config, "addons.filetreediff.diff"); | |
const diffData = objectPath.get(this.config, "addons.filetreediff.diff"); |
Added a new addon to show all the filenames and URLs that were added, deleted or modified in the current PR compared to the LATEST version.
I'm rendering this data in a new notification decoupled from the existing one on purpose. I don't want to entangle the code of these two different addons for now, in particular while we are under development/testing. I also see that in the future they may have different UI elements, or the data will be integrated in a more bigger UI element (eg. toolbar) or similar.
We can discuss more how this addon will grow and where is a good place to add this data and make that work in a following PR. For now, I want something that we can start testing internally in our own projects.
Examples
Closes #409
Requires readthedocs/readthedocs.org#11749