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

meta: Postpone non-breaking TODOs #15080

Merged
merged 3 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/aws-serverless/src/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ export function wrapHandler<TEvent, TResult>(
// Only start a trace and root span if the handler is not already wrapped by Otel instrumentation
// Otherwise, we create two root spans (one from otel, one from our wrapper).
// If Otel instrumentation didn't work or was filtered by users, we still want to trace the handler.
// TODO(v9): Since bumping the OTEL Instrumentation, this is likely not needed anymore, we can possibly remove this
// TODO(v9): Since bumping the OTEL Instrumentation, this is likely not needed anymore, we can possibly remove this (can be done whenever since it would be non-breaking)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just remove the (v9) suffix here, possibly? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I guess - I originally wanted to keep it because it's a somewhat valuable priority gauge when grepping for TODO(

if (options.startTrace && !isWrappedByOtel(handler)) {
const traceData = getAwsTraceData(event as { headers?: Record<string, string> }, context);

Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,9 +122,9 @@ export type { ReportDialogOptions } from './report-dialog';
// eslint-disable-next-line deprecation/deprecation
export { getCurrentHubShim, getCurrentHub } from './getCurrentHubShim';

// TODO(v9): Make this structure pretty again and don't do "export *"
// TODO(v9): Make this structure pretty again and don't do "export *" (since this is non-breaking, can be done whenever)
export * from './utils-hoist/index';
// TODO(v9): Make this structure pretty again and don't do "export *"
// TODO(v9): Make this structure pretty again and don't do "export *" (since this is non-breaking, can be done whenever)
export * from './types-hoist/index';

export type { FeatureFlag } from './featureFlags';
2 changes: 1 addition & 1 deletion packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,7 @@ function setUpModuleRules(newConfig: WebpackConfigObject): WebpackConfigObjectWi
/**
* Adds loaders to inject values on the global object based on user configuration.
*/
// TODO(v9): Remove this loader and replace it with a nextConfig.env (https://web.archive.org/web/20240917153554/https://nextjs.org/docs/app/api-reference/next-config-js/env) or define based (/~https://github.com/vercel/next.js/discussions/71476) approach.
// TODO: Remove this loader and replace it with a nextConfig.env (https://web.archive.org/web/20240917153554/https://nextjs.org/docs/app/api-reference/next-config-js/env) or define based (/~https://github.com/vercel/next.js/discussions/71476) approach.
// In order to remove this loader though we need to make sure the minimum supported Next.js version includes this PR (/~https://github.com/vercel/next.js/pull/61194), otherwise the nextConfig.env based approach will not work, as our SDK code is not processed by Next.js.
function addValueInjectionLoader(
newConfig: WebpackConfigObjectWithModuleRules,
Expand Down
4 changes: 1 addition & 3 deletions packages/nextjs/src/config/withSentryConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,9 +248,7 @@ function setUpTunnelRewriteRules(userNextConfig: NextConfigObject, tunnelPath: s
};
}

// TODO(v9): Inject the release into all the bundles. This is breaking because grabbing the build ID if the user provides
// it in `generateBuildId` (https://nextjs.org/docs/app/api-reference/next-config-js/generateBuildId) is async but we do
// not turn the next config function in the type it was passed.
// TODO: For Turbopack we need to pass the release name here and pick it up in the SDK
function setUpBuildTimeVariables(userNextConfig: NextConfigObject, userSentryOptions: SentryBuildOptions): void {
const assetPrefix = userNextConfig.assetPrefix || userNextConfig.basePath || '';
const basePath = userNextConfig.basePath ?? '';
Expand Down
Loading