sphinx-contrib / images

sphinxcontrib-images extension
Apache License 2.0
17 stars 14 forks source link

Fix #8: Incremental building not working #14

Closed kujiu closed 3 years ago

kujiu commented 4 years ago

Changing config parameters after config provokes regenerating all files. This PR update config parameters during config-inited event.

jonascj commented 4 years ago

Thank you, at first glance it looks good. I have however not dealt with the module configuration before so I will need some time to replicate your findings from Issue https://github.com/sphinx-contrib/images/issues/8 and review the changes proposed in this PR.

I plan on having it done this week. Do you think we should make a new release with this fix?

kujiu commented 4 years ago

I think it could be a good idea. I have no plan to code during next weeks for this project (I need more time...).

jonascj commented 3 years ago

How time flies when you have a baby and a 3-year-old! Please accept my apologies for not attending to the PRs and issues in a timely fashion. I'll attend to them at the latest on August 29th and 30th (I have another project with deadline on August 28th).

terencehonles commented 3 years ago

How time flies when you have a baby and a 3-year-old! Please accept my apologies for not attending to the PRs and issues in a timely fashion. I'll attend to them at the latest on August 29th and 30th (I have another project with deadline on August 28th).

@jonascj not sure @kujiu has had a chance to review my suggestions, but if you'd like me to add my suggestions to another branch and create a PR off of that I can do so. I'd leave attribution for @kujiu so they'll get credit for their work.

kujiu commented 3 years ago

Sorry, I didn't see the notification for comments. I don't have time this week and perhaps the next too, but I don't care about credits. You can do the stuff and erase my contribution if you want. Sorry, I have really big projects.

jonascj commented 3 years ago

@terencehonles I just meant I'll go over the PR/issue discussions, start reviewing contributions etc. I have no desire to redo good work already done. So if @kujiu has found a good, viable solution we'll use it and credit him one way or another, don't worry.

terencehonles commented 3 years ago

Sorry, I didn't see the notification for comments. I don't have time this week and perhaps the next too, but I don't care about credits. You can do the stuff and erase my contribution if you want. Sorry, I have really big projects.

No worries @kujiu , I wasn't sure if you were checking your github notifications often. I updated the commit here: https://github.com/terencehonles/sphinx-contrib-images/commit/f1184cd2c517465b5f5fd9e54fe06745d9557c43

You can quickly update this PR with the following commands:

# fetch from my repo without adding another remote
git fetch git@github.com:terencehonles/sphinx-contrib-images.git master-fix-incremental-build

# take what you just fetched and push it to the branch this PR is tracking
git push origin +FETCH_HEAD:master-fix-incremental-build

@terencehonles I just meant I'll go over the PR/issue discussions, start reviewing contributions etc. I have no desire to redo good work already done. So if @kujiu has found a good, viable solution we'll use it and credit him one way or another, don't worry.

@jonascj the suggestions are not re-doing anything, just clarifying what the plugin is doing with respect to the changes in the PR

SilverRainZ commented 3 years ago

Any progress?

terencehonles commented 3 years ago

Any progress?

I opened a PR (#18) with my suggestions, but either way we'll need a little time from @jonascj to test and merge them.

jonascj commented 3 years ago

@terencehonles Thank you for moving it forward, I'll try to look at it this coming weekend (9th, 10th January).

@SilverRainZ When you asked there was no new progress, no, but now there is a little progress.

See also #20 :-)

jonascj commented 3 years ago

Merged #18 which was based on this #14.

terencehonles commented 3 years ago

sorry I had too many tabs open and looks like I put it on the wrong one :facepalm: it's fixed now