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

feat: --tag parameter #196 #282

Merged
merged 7 commits into from Nov 5, 2022
Merged

feat: --tag parameter #196 #282

merged 7 commits into from Nov 5, 2022

Conversation

imbdb
Copy link
Contributor

@imbdb imbdb commented Oct 30, 2022

Details

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I checked the current PR for duplication.

@imbdb
Copy link
Contributor Author

imbdb commented Oct 30, 2022

PlasmoHQ/plasmo-utils#1
PR for utils package to add --tag

@louisgv
Copy link
Contributor

louisgv commented Nov 1, 2022

@imbdb thanks for the PR! I will season this a bit and merge it this week. I think that the util common module doesn't need to be updated as it's only for the most common. Application specific flag (like the --tag) should stays locally. Great work nonetheless :)

@imbdb
Copy link
Contributor Author

imbdb commented Nov 1, 2022

@louisgv Thanks for the comment, I have closed the utils PR.
Waiting for the feature...

@louisgv louisgv changed the title Feat: --tag parameter #196 feat: --tag parameter #196 Nov 5, 2022
@@ -35,7 +35,7 @@ export const getCommonPath = (

const distDirectoryName = `${target}-${
process.env.NODE_ENV === "production" ? "prod" : "dev"
}`
}${tag ? '-' + tag : ''}`
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we would replace the dev/prod entirely if a tag is present. The idea is that prod and dev are technically build tag. You can use the tag flag for both dev and prod bundle, it just influence the naming of this directory.

Copy link
Contributor

@louisgv louisgv left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@louisgv louisgv self-assigned this Nov 5, 2022
@louisgv louisgv merged commit f4ed50f into PlasmoHQ:main Nov 5, 2022
@louisgv
Copy link
Contributor

louisgv commented Nov 6, 2022

@imbdb I published this in canary, pls try it out when you got a chance :)

@imbdb
Copy link
Contributor Author

imbdb commented Nov 11, 2022

Thanks @louisgv
I will check this weekend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

[RFC] build --tag=staging
2 participants