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 unpopulated option to additional components qty multiplier #298

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

goopypanther
Copy link
Contributor

qty_multiplier can be set to unpopulated when defining additional connector components. This will calculate the number of empty pin spaces in the connector.

I use this for waterproof connectors which require sealing plugs to be installed in unoccupied sockets.

@goopypanther goopypanther changed the base branch from master to dev December 12, 2022 02:26
@goopypanther goopypanther changed the base branch from dev to master December 12, 2022 02:28
@kvid
Copy link
Collaborator

kvid commented Sep 14, 2023

@JeremyRuhland - Thank you for your suggested feature implementation, and I wonder why you closed this PR just a few hours after creating it in December? I find your use case valuable and your implementation fit well together with existing features.

I can see that you changed the base branch from master to dev and then back again just a few minutes after creating the PR, and I agree that dev should be the base branch. Maybe you discovered warnings about conflicts after changing to dev and therefore changed back to avoid the conflicts. Such conflicts appear because there are a lot of commits in the dev branch that are not yet in the master branch, and I recommend rebasing your feature branch on top of the current dev branch:

  • Any uncommitted changes in your workspace should be committed or stashed away before rebasing.
  • The command git rebase --onto dev master unpop_addit_cmp will try to replay on top of dev all your changes in unpop_addit_cmp that is currently on top of master. Often, git manages to do most of this automatically, but in your case two conflicts had to be resolved during the rebase process. See the result in my fork and verify that it still contains your intended changes.
  • After such a rebasing, the rebased branch must be force-pushed to your fork (git push -f) to update the PR.
  • Then the base branch must be set to dev and there should be no more conflicts until someone add more commits to dev again. When that happens, the process must be repeated once more, but the number of conflicts is normally less this time.

Unless you rather want to do the whole process yourself, I can normally push my rebase result onto your fork to save you the work I have already done (unless you unchecked "Allow edits and access to secrets by maintainers" when creating your PR). If the weeks go without you responding here, I might just push my result to your fork to be able to make a proper review of your PR. The push and change of base branch is now done.

Edit: Remember to add an entry about this PR into the CHANGELOG before any merge-in.

Edit2: Test what happens in the rare edge case when pincount is set lower than the number of populated pins - that is theoretically possible when the highest actual number of entries in pins, pinlabels, or pincolors is higher than pincount. IMHO, a negative qty_multiplier does not make sense, so I suggest limiting the value to minimum zero. See #285 (comment) for a use case where this might happen.

@kvid kvid reopened this Sep 14, 2023
@kvid kvid changed the base branch from master to dev November 11, 2023 00:23
src/wireviz/DataClasses.py Outdated Show resolved Hide resolved
@formatc1702 formatc1702 merged commit f98bf2a into wireviz:dev Apr 16, 2024
2 checks passed
@formatc1702
Copy link
Collaborator

Changelog updated immediately post-merge.

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.

3 participants