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

IContainerContext removals #16262

Merged
merged 6 commits into from
Jul 7, 2023

Conversation

ChumpChief
Copy link
Contributor

@ChumpChief ChumpChief commented Jul 6, 2023

Followups from #16180 and #16182. AB#4879

@ChumpChief ChumpChief requested review from a team as code owners July 6, 2023 18:53
@github-actions github-actions bot added the public api change Changes to a public API label Jul 6, 2023
@github-actions github-actions bot added the base: next PRs targeted against next branch label Jul 6, 2023
@ChumpChief
Copy link
Contributor Author

@tylerbutler the changes to UPCOMING.md look more dramatic than I expected - wanted to make sure I'm not doing something wrong here.

@msfluid-bot
Copy link
Collaborator

msfluid-bot commented Jul 6, 2023

@fluid-example/bundle-size-tests: -404 Bytes
Metric NameBaseline SizeCompare SizeSize Diff
aqueduct.js 432.4 KB 432.4 KB No change
connectionState.js 680 Bytes 680 Bytes No change
containerRuntime.js 231.97 KB 231.97 KB No change
loader.js 144.48 KB 144.08 KB -404 Bytes
map.js 44.88 KB 44.88 KB No change
matrix.js 138.07 KB 138.07 KB No change
odspDriver.js 87.82 KB 87.82 KB No change
odspPrefetchSnapshot.js 41.12 KB 41.12 KB No change
sharedString.js 153.93 KB 153.93 KB No change
sharedTree2.js 236.35 KB 236.35 KB No change
Total Size 1.63 MB 1.63 MB -404 Bytes

Baseline commit: da246ca

Generated by 🚫 dangerJS against d1cb575

@ChumpChief ChumpChief requested a review from a team as a code owner July 6, 2023 19:22
@github-actions github-actions bot added the area: loader Loader related issues label Jul 6, 2023
@ChumpChief
Copy link
Contributor Author

Forgot that we can also remove the implementations here too, removed in latest.

@ChumpChief
Copy link
Contributor Author

I see the policy check is running a "generate minor" but I think it's appropriate for it to instead generate major since this is targeting next branch?

@tylerbutler
Copy link
Member

I see the policy check is running a "generate minor" but I think it's appropriate for it to instead generate major since this is targeting next branch?

Working as expected right now, but it looks like next may have some unexpected changesets. I'll look more at it later, but once the latest main is merged (#16275), let's see what things look like.

@tylerbutler
Copy link
Member

@ChumpChief Try merging the latest next into this PR; I think that should improve the UPCOMING.md situation, but I still need to figure out how to deal with main vs. next more generally.

@ChumpChief
Copy link
Contributor Author

@ChumpChief Try merging the latest next into this PR; I think that should improve the UPCOMING.md situation, but I still need to figure out how to deal with main vs. next more generally.

Merged latest, but I think UPCOMING.md looks wrong in current next, doesn't include all the major changes that have happened in next. If I run the command with the major flag it looks more like I'd expect (including the changeset for this PR).

"generate:upcoming": "flub generate upcoming --releaseGroup client -t minor major"

@github-actions
Copy link
Contributor

github-actions bot commented Jul 7, 2023

Please review this PR: @sonalideshpandemsft @tylerbutler @scottn12

@ChumpChief
Copy link
Contributor Author

Tagging merge-next since I think the UPCOMING behavior is consistent with what's in next currently, but probably something to figure out there still.

@tylerbutler
Copy link
Member

"generate:upcoming": "flub generate upcoming --releaseGroup client -t minor major"

Yes, what you're describing is the expected behavior. UPCOMING.md always contains ONLY minor changes. That's what I meant by "figure out how to deal with main vs. next more generally."

@tylerbutler tylerbutler merged commit 3fb1e20 into microsoft:next Jul 7, 2023
30 checks passed
@ChumpChief ChumpChief deleted the IContainerContextRemovals branch July 7, 2023 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: loader Loader related issues base: next PRs targeted against next branch msftbot: merge-next public api change Changes to a public API
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants