extensions/add_mesh_BoltFactory
1
1
Fork 4
Code Issues 5 Pull Requests 2 Projects Releases Wiki Activity

Fix for #8 'Operator Presets don't exists from Release 4.2' #11

Merged
Nika Kutsniashvili merged 2 commits from linux_dr/add_mesh_BoltFactory:losborn_ISSUE8_missing_presets into main 2024-08-20 22:39:56 +02:00
Conversation 12 Commits 2 Files Changed 9 +308
Loren Osborn commented 2024-08-15 02:56:41 +02:00
Contributor
Copy Link

Fix for #8 : Restored BoltFactory presets with proper scaling, based on suggestions from @sw-tya and code from 73c82edab2

Fix for https://projects.blender.org/extensions/add_mesh_BoltFactory/issues/8 : Restored BoltFactory presets with proper scaling, based on suggestions from @sw-tya and code from https://projects.blender.org/extensions/add_mesh_extra_objects/commit/73c82edab2969bb19d14416a0931ff51b0c2b382
Loren Osborn added 2 commits 2024-08-15 02:56:45 +02:00 
#8
Loren Osborn changed title from losborn_ISSUE8_missing_presets to Fix for #8 'Operator Presets don't exists from Release 4.2' 2024-08-15 17:53:12 +02:00
sw-tya reviewed 2024-08-16 02:42:01 +02:00
sw-tya left a comment
First-time contributor
Copy Link

I need more time to complete the review.
So far i can see that the presets appear in the GUI list as expected. I'm testing using a local build of 4.3.0 Alpha.
Having the default as 12.5 for the Crest_Percent means that the #10 is required as well. This confused me for a while as with:
"git fetch origin pull/11/head:losborn_ISSUE8_missing_presets"
"git checkout losborn_ISSUE8_missing_presets"
The changes with the Float had gone from my folder.
I've got to do more learning on how to see both pulls at the same time (assuming that is possible)...

I need more time to complete the review. So far i can see that the presets appear in the GUI list as expected. I'm testing using a local build of 4.3.0 Alpha. Having the default as 12.5 for the Crest_Percent means that the #10 is required as well. This confused me for a while as with: "git fetch origin pull/11/head:losborn_ISSUE8_missing_presets" "git checkout losborn_ISSUE8_missing_presets" The changes with the Float had gone from my folder. I've got to do more learning on how to see both pulls at the same time (assuming that is possible)...
Loren Osborn commented 2024-08-16 03:33:38 +02:00
Author
Contributor
Copy Link

Yes, good catch. I hadn't realized that I had separated out the change from IntProperty to FloatProperty from the change of 10 to 12.5 in the presets, but I obviously had. I would not do a final approval of this until #10 is merged in, and I can rebase this PR on the merge... That said, here is how to test both fixes together in the meantime:

# Checkout the branch with the changes that should "happen last"
git checkout pull/11/head:losborn_ISSUE8_missing_presets
# Create a new branch where both fixes can live together
git checkout -b swtya_combinedFixes_PR10_and_PR11
# Rebase the changes from losborn_ISSUE8_missing_presets onto the changes in pull/10/head:losborn_PR5_FIXUP_scale
git rebase pull/10/head:losborn_PR5_FIXUP_scale

now you will have a new local branch named swtya_combinedFixes_PR10_and_PR11, with changes from both branches available to test simultaneously

Thank you for your astute catches

Yes, good catch. I hadn't realized that I had separated out the change from `IntProperty` to `FloatProperty` from the change of `10` to `12.5` in the presets, but I obviously had. I would not do a **final approval** of this until #10 is merged in, and I can rebase this PR on the merge... That said, here is how to test both fixes together in the meantime: ``` # Checkout the branch with the changes that should "happen last" git checkout pull/11/head:losborn_ISSUE8_missing_presets # Create a new branch where both fixes can live together git checkout -b swtya_combinedFixes_PR10_and_PR11 # Rebase the changes from losborn_ISSUE8_missing_presets onto the changes in pull/10/head:losborn_PR5_FIXUP_scale git rebase pull/10/head:losborn_PR5_FIXUP_scale ``` now you will have a new local branch named `swtya_combinedFixes_PR10_and_PR11`, with changes from both branches available to test simultaneously Thank you for your astute catches
sw-tya approved these changes 2024-08-17 00:44:04 +02:00
sw-tya left a comment
First-time contributor
Copy Link

After I'd got #10 and #11 co-resident, thanks:
Tested with production Blender LTS 4.2.0 and home build 4.3.0
4.2.0 is not any worse , that's still awaiting the directory search backport.
4.3.0 works correctly. Presets all load, sizes are good. I checked against an old folder I had from a different version - files seem consistent only expected changes present.
Ship it.

After I'd got #10 and #11 co-resident, thanks: Tested with production Blender LTS 4.2.0 and home build 4.3.0 4.2.0 is not any worse ~~, that's still awaiting the directory search backport.~~ 4.3.0 works correctly. Presets all load, sizes are good. I checked against an old folder I had from a different version - files seem consistent only expected changes present. Ship it.
Loren Osborn commented 2024-08-17 04:28:13 +02:00
Author
Contributor
Copy Link

@sw-tya wrote:

…, that's still awaiting the directory search backport.
4.3 works correctly.

I don’t know anything about this directory search feature you mentioned. Can you please write a bug report issue for it, Including how it’s supposed to behave and steps to reproduce.

I’m unsure if I’ll be able to address it but at least that way it will be documented and tracked.

@sw-tya wrote: >…, that's still awaiting the directory search backport. > 4.3 works correctly. I don’t know anything about this directory search feature you mentioned. Can you please write a bug report issue for it, Including how it’s supposed to behave and steps to reproduce. I’m unsure if I’ll be able to address it but at least that way it will be documented and tracked.
Loren Osborn commented 2024-08-17 04:36:42 +02:00
Author
Contributor
Copy Link

@nickberckley, this should be ready to rebase and merge after #10 (@sw-tya found a dependency on #10 that I missed.)

@nickberckley, this should be ready to rebase and merge after #10 (@sw-tya found a dependency on #10 that I missed.)
sw-tya commented 2024-08-17 21:43:51 +02:00
First-time contributor
Copy Link

@sw-tya wrote:

…, that's still awaiting the directory search backport.
4.3 works correctly.

I don’t know anything about this directory search feature you mentioned. Can you please write a bug report issue for it, Including how it’s supposed to behave and steps to reproduce.

I’m unsure if I’ll be able to address it but at least that way it will be documented and tracked.

4.2.0 does not support any extension shipping with presets.
23fc0d53e1
is apparently (blender/blender#124452) in the 4.2.1 list for when that ships.
This is what I have gathered from trawling https://projects.blender.org/
I don't believe anything more can be done from the extensions world.

> @sw-tya wrote: > >…, that's still awaiting the directory search backport. > > 4.3 works correctly. > > I don’t know anything about this directory search feature you mentioned. Can you please write a bug report issue for it, Including how it’s supposed to behave and steps to reproduce. > > I’m unsure if I’ll be able to address it but at least that way it will be documented and tracked. 4.2.0 does not support any extension shipping with presets. https://projects.blender.org/blender/blender/commit/23fc0d53e1458dc6a1d8072f77325a70416bd4a9 is apparently (https://projects.blender.org/blender/blender/issues/124452) in the 4.2.1 list for when that ships. This is what I have gathered from trawling https://projects.blender.org/ I don't believe anything more can be done from the extensions world.
Nika Kutsniashvili commented 2024-08-20 16:25:01 +02:00
Owner
Copy Link

@linux_dr can you merge main in this PR branch?
Also remove comments about 4.3 might be backported to 4.2

@linux_dr can you merge main in this PR branch? Also remove comments about 4.3 might be backported to 4.2
Loren Osborn commented 2024-08-20 19:24:58 +02:00
Author
Contributor
Copy Link

@nickberckley wrote:

@linux_dr can you merge main in this PR branch?

I have no permissions to merge any PRs. If someone wants to grant me such permissions we can stop asking you.

Update: I apologize that I misread your reply… I expect you meant for me to rebase #11 onto main… I hope to do that very soon… (I think i also confused #10 and #11)… I’ll get right on this.

Also remove comments about 4.3 might be backported to 4.2

@sw-tya is the only one that mentioned 4.2 and 4.3… I’ve only been working off main

@nickberckley wrote: > @linux_dr can you merge main in this PR branch? I have no permissions to merge any PRs. If someone wants to grant me such permissions we can stop asking you. **Update:** I apologize that I misread your reply… I expect you meant for me to rebase #11 onto `main`… I hope to do that very soon… (I think i also confused #10 and #11)… I’ll get right on this. > Also remove comments about 4.3 might be backported to 4.2 @sw-tya is the only one that mentioned 4.2 and 4.3… I’ve only been working off `main`
Nika Kutsniashvili commented 2024-08-20 19:40:09 +02:00
Owner
Copy Link

No need to rebase anything, just in your local branch do

git fetch origin main
git pull origin main

there shouldn't be any merge conflicts afaik. And when you're done (and remove those comments) just push normally

git push your-remote-name your-branch-name
No need to rebase anything, just in your local branch do ``` git fetch origin main git pull origin main ``` there shouldn't be any merge conflicts afaik. And when you're done (and remove those comments) just push normally ``` git push your-remote-name your-branch-name ```
Loren Osborn force-pushed losborn_ISSUE8_missing_presets from 32ab195379 to f2d3ab9487 2024-08-20 20:19:22 +02:00 Compare
Loren Osborn commented 2024-08-20 20:25:53 +02:00
Author
Contributor
Copy Link

@sw-tya wrote:

4.2 is not any worse, that's still awaiting the directory search backport.

could you edit this comment to make it something like:

4.2 is not any worse, that's still awaiting the directory search backport.

Just add a ~~ at the beginning and end of the line

@sw-tya wrote: > 4.2 is not any worse, that's still awaiting the directory search backport. could you edit this comment to make it something like: 4.2 is not any worse, ~~that's still awaiting the directory search backport.~~ Just add a `~~` at the beginning and end of the line
sw-tya commented 2024-08-20 21:18:16 +02:00
First-time contributor
Copy Link

The text:
# Part of 4.3 may be back-ported to 4.2.
was copied from the Cambell Barton in:
73c82edab2

I've clarified my test versions and struck text in my previous comment.

The text: ` # Part of 4.3 may be back-ported to 4.2.` was copied from the Cambell Barton in: https://projects.blender.org/extensions/add_mesh_extra_objects/commit/73c82edab2969bb19d14416a0931ff51b0c2b382 I've clarified my test versions and struck text in my previous comment.
Nika Kutsniashvili commented 2024-08-20 22:39:26 +02:00
Owner
Copy Link

I'll merge this and deal with any related future changes myself

I'll merge this and deal with any related future changes myself
Nika Kutsniashvili merged commit 1944834b0e into main 2024-08-20 22:39:56 +02:00
Loren Osborn commented 2024-08-24 18:04:38 +02:00
Author
Contributor
Copy Link

Is there a reason this isn't closed yet? Is someone waiting on me to do something?

Is there a reason this isn't closed yet? Is someone waiting on me to do something?
Nika Kutsniashvili commented 2024-08-24 21:58:33 +02:00
Owner
Copy Link

This is closed

This is closed
Sign in to join this conversation.
Reviewers
No reviewers
sw-tya
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: extensions/add_mesh_BoltFactory#11
No description provided.
Delete Branch "linux_dr/add_mesh_BoltFactory:losborn_ISSUE8_missing_presets"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?