Add --development-context flag to theme push command
#6657
Conversation
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
graygilmore
force-pushed
the
gg-push-with-name
branch
from
9a88cb6 to
367cc05
Compare
Contributor
Coverage report
Show new covered files 🐣
Show files with reduced coverage 🔻
Test suite run success3730 tests passing in 1445 suites. Report generated by 🧪jest coverage report action from 825b91e |
Contributor
|
This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. |
graygilmore
force-pushed
the
gg-push-with-name
branch
from
367cc05 to
c0dd1ba
Compare
graygilmore
force-pushed
the
gg-push-with-name
branch
2 times, most recently
from
8b81097 to
c24eece
Compare
graygilmore
changed the title
--name flag to theme push command--development-context flag to theme push command
graygilmore
force-pushed
the
gg-push-with-name
branch
4 times, most recently
from
baa67ff to
8a26885
Compare
Contributor
Author
|
/snapit |
Contributor
|
🫰✨ Thanks @graygilmore! Your snapshot has been published to npm. Test the snapshot by installing your package globally: npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]Caution After installing, validate the version by running just |
graygilmore
force-pushed
the
gg-push-with-name
branch
from
8a26885 to
825b91e
Compare
Contributor
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationspackages/cli-kit/dist/cli/api/graphql/admin/generated/find_development_theme_by_name.d.tsimport * as Types from './types.js';
import { TypedDocumentNode as DocumentNode } from '@graphql-typed-document-node/core';
export type FindDevelopmentThemeByNameQueryVariables = Types.Exact<{
name: Types.Scalars['String']['input'];
}>;
export type FindDevelopmentThemeByNameQuery = {
themes?: {
nodes: {
id: string;
name: string;
role: Types.ThemeRole;
processing: boolean;
}[];
} | null;
};
export declare const FindDevelopmentThemeByName: DocumentNode<FindDevelopmentThemeByNameQuery, FindDevelopmentThemeByNameQueryVariables>;
Existing type declarationspackages/cli-kit/dist/public/node/themes/api.d.ts@@ -5,6 +5,7 @@ export type ThemeParams = Partial<Pick<Theme, 'name' | 'role' | 'processing' | '
export type AssetParams = Pick<ThemeAsset, 'key'> & Partial<Pick<ThemeAsset, 'value' | 'attachment'>>;
export declare function fetchTheme(id: number, session: AdminSession): Promise<Theme | undefined>;
export declare function fetchThemes(session: AdminSession): Promise<Theme[]>;
+export declare function findDevelopmentThemeByName(name: string, session: AdminSession): Promise<Theme | undefined>;
export declare function themeCreate(params: ThemeParams, session: AdminSession): Promise<Theme | undefined>;
export declare function fetchThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<ThemeAsset[]>;
export declare function deleteThemeAssets(id: number, filenames: Key[], session: AdminSession): Promise<Result[]>;
packages/cli-kit/dist/public/node/themes/theme-manager.d.ts@@ -8,8 +8,8 @@ export declare abstract class ThemeManager {
protected abstract removeTheme(): void;
protected abstract context: string;
constructor(adminSession: AdminSession);
- findOrCreate(): Promise<Theme>;
- fetch(): Promise<Theme | undefined>;
+ findOrCreate(name?: string): Promise<Theme>;
+ fetch(name?: string): Promise<Theme | undefined>;
generateThemeName(context: string): string;
create(themeRole?: Role, themeName?: string): Promise<Theme>;
}
\ No newline at end of file
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
WHY are these changes introduced?
Developers often want to create a new development theme on a per-PR basis when reviewing theme changes. This is difficult right now because our systems try to only have one development theme per user which means that GitHub's CI is considered a single user and development themes get overwritten all the time.
WHAT is this pull request doing?
This PR solves this by allowing developers to set a
--development-contextflag when usingtheme push --developmentto set a "context" for a development theme to be pushed to. The context in this case is just a name but we've called it--development-contextinstead of--namebecause it's specific to development themes and we don't want to confuse with the--themeflag which accepts both an ID and a name.When a user sets
--development-contextwe'll search for all development themes with that name. If we find one we'll push to it otherwise we'll create a new development themes. This lets developers decide whether to keep long running contexts that get overwritten (e.g.PR #<number>) or new development themes on every push (e.g.PR #<number> - <latest_sha>).How to test your changes?
npm i -g --@shopify:registry=https://registry.npmjs.org @shopify/[email protected]shopify theme list --role developmentto see all of your development themesshopify theme push -d -c test-1shopify theme list --role developmentagain and ensure you see the new themeshopify theme push -d -c test-2shopify theme list --role developmentagain and ensure a new theme is created (ie, the previous development theme isn't replaced)shopify theme push -d -c test-1againshopify theme list --role developmentagain and ensure test-1 was replaced and there aren't two themes named test-1