-
Notifications
You must be signed in to change notification settings - Fork 450
fix: suggested fixes for #3514 #3538
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
Closed
NoelStephensUnity
wants to merge
31
commits into
fix/destroy-in-scene-placed-objects
from
fix/destroy-in-scene-placed-objects-da-fixes
Closed
Changes from all commits
Commits
Show all changes
31 commits
Select commit
Hold shift + click to select a range
30cf113
ci: [NGOv2.X]Updated Wrench and added project dependency (#3512)
michalChrobot b87dae7
feat: Expose methods for ClientId to TransportId mappings (#3516)
EmandM 1be4c4a
Merge branch 'develop-2.0.0' of https://github.com/Unity-Technologies…
EmandM 27bae93
fix
NoelStephensUnity 3a64ab0
chore: Add tech writer to codeowners 2.x (#3525)
jabbacakes 3632713
Merge develop-2.0.0 into fix/destroy-in-scene-placed-objects
netcode-ci-service c8fb89e
chore: merge v2.4.3 back into develop-2.0.0 (#3523)
NoelStephensUnity 728eb24
Merge develop-2.0.0 into fix/destroy-in-scene-placed-objects
netcode-ci-service 098e68e
ci: Integrating CodeCov with NGO repo [develop-2.0.0] (#3474)
michalChrobot 88b3a07
ci: [NGOv2.X] Fixing WebGL timeout and reenabling macOS job (#3507)
michalChrobot ff571bc
chore(deps): update dependency recipeengine.modules.wrench to 0.12.2 …
unity-renovate[bot] a06c9d8
docs: Adding docs to the 2.4 version of the package (#3521)
jabbacakes 9e2e880
Ensure all clients can re-parent objects in DA mode
EmandM 424fbeb
rename test network objects
EmandM 10f2a51
fix/mttb-1377/synchronize-despawn-to-client
EmandM 33bcbcc
Update changelog
EmandM ba62b90
dotnet-fix
EmandM 48bb41b
InScenePlacedNetworkObjectTests rework
EmandM 921707f
style
NoelStephensUnity b609e69
fixes
EmandM 147bf38
fix
NoelStephensUnity 99d5043
Ensure all clients can re-parent objects in DA mode
EmandM 0ea6a8e
rename test network objects
EmandM 1e2a5c9
Update NetworkShowHideTests to work with cmb service
EmandM 41fc2c8
Merge branch 'fix/destroy-in-scene-placed-objects' of https://github.…
EmandM 56e1b6f
fix
NoelStephensUnity 21d8fcf
Merge branch 'fix/destroy-in-scene-placed-objects' of https://github.…
NoelStephensUnity 944087b
style
NoelStephensUnity 340549c
fix
NoelStephensUnity 8c67ce3
update
NoelStephensUnity 452a9c4
Fix-PVP
NoelStephensUnity File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
codecov: | ||
require_ci_to_pass: false | ||
notify: | ||
wait_for_ci: true | ||
|
||
coverage: | ||
precision: 2 | ||
round: down | ||
range: "50...70" | ||
status: | ||
patch: | ||
default: | ||
if_ci_failed: success | ||
informational: true | ||
default_rules: | ||
flag_coverage_not_uploaded_behavior: exclude | ||
project: | ||
default: | ||
target: auto | ||
# Threshold used for the PR Check | ||
threshold: 0.5% | ||
base: auto | ||
if_ci_failed: success | ||
informational: true | ||
only_pulls: true | ||
|
||
# PR Comment configuration | ||
comment: | ||
layout: "diff, flags, files, components" | ||
behavior: default | ||
require_changes: false | ||
require_base: false | ||
require_head: false | ||
# Set this to the number of coverage jobs run in the PR | ||
after_n_builds: 1 | ||
|
||
flag_management: | ||
default_rules: | ||
carryforward: true | ||
|
||
# Components configuration | ||
component_management: | ||
individual_components: | ||
- component_id: "com.unity.netcode.gameobjects" | ||
name: "com.unity.netcode.gameobjects" | ||
paths: | ||
- com.unity.netcode.gameobjects |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -66,6 +66,7 @@ pull_request_trigger: | |
- "develop" | ||
- "develop-2.0.0" | ||
- "/release\/.*/" | ||
- "/fix\/.*/" | ||
- drafts: false | ||
|
||
|
||
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Why to add this automatically? My understanding is that we don't need to "waste resources" and enforce the test passing for PRs that are not "landing". So what I would do is to trigger those tests manually in Yamato if you want to ensure that it's passing.
Otherwise notice that if there is a PR targeting develop branch to which you are merging then after the merge the tests will automatically run on that branch
Also fix/ may be a bit general in a way that someone may be surprised by enforced tests (in my opinion)
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.
Adding /fix/. to the triggers should have zero impact on triggering anything on the develop branch.
For fixes, using "fix/" as the prefix to a branch name is actually the standard.... fix/
Just like for chores, using "chore" as the prefix to a branch name is actually the standard.... chore/
But all of this is just my recommended updates/fixes and we can just as easily close the PR and delete the branch.
Uh oh!
There was an error while loading. Please reload this page.
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.
Maybe I actually described it a bit wrongly, but my point is just that this job will be triggered twice. Since the trigger definition will be
This mean that this entire set of jobs will run on ANY PR that targets
fix/
... branch. Assumption is that saidfix/
branch will either targetdevelop
/develop-2.0.0
(in that case this set of jobs will run as per job trigger definition) or it will merge to some other branch that at some point in the future will targetdevelop
/develop-2.0.0
(since otherwise it will never be part of the package) and in that point this set of jobs will runSo that's why I'm more into "If your branch is not targeting
develop
/develop-2.0.0
then we don't force running those jobs in order for the PR to merge"