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

Move to default branch #7231

Draft
wants to merge 12 commits into
base: master
Choose a base branch
from
Draft

Move to default branch #7231

wants to merge 12 commits into from

Conversation

pelikhan
Copy link
Member

We shouldn't assume that master is the default branch. Cleaning up to remove this assumption.

@pelikhan
Copy link
Member Author

pelikhan commented Aug 4, 2020 via email

@pelikhan pelikhan marked this pull request as draft August 7, 2020 04:27
@abchatra
Copy link
Contributor

abchatra commented Nov 2, 2020

@pelikhan what is pending in this PR?
@aznhassan

@pelikhan
Copy link
Member Author

pelikhan commented Nov 3, 2020

Backend need to be able to pull the default branch when not specified.

Switch template to use "main" be default?

Otherwise lots of testing.

Copy link
Member Author

@pelikhan pelikhan left a comment

Choose a reason for hiding this comment

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

When we load a configuration from the cloud, we don't know the default branch. The could should support a tag called "default" and use "master" or "main" as needed.

export function isDefaultBranch(branch: string, repo?: GitRepo) {
if (repo && repo.defaultBranch)
return branch === repo.defaultBranch;
return /^(main|master)$/.test(branch);
Copy link
Member Author

Choose a reason for hiding this comment

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

Dubious...

@pelikhan pelikhan requested a review from jwunderl May 11, 2021 18:46
const defaultBranch = pxt.github.isDefaultBranch(branch)
const latest = defaultBranch ? "latest" : "git-" + branch
// upload locs on build on default or releases
const apiStringBranchRx: (t: string) => boolean = ((t:string) => new RegExp(pxt.appTarget.uploadApiStringsBranchRx).test(t))
Copy link
Member

@jwunderl jwunderl May 11, 2021

Choose a reason for hiding this comment

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

Suggested change
const apiStringBranchRx: (t: string) => boolean = ((t:string) => new RegExp(pxt.appTarget.uploadApiStringsBranchRx).test(t))
const apiStringBranchRx: (t: string) => boolean = ((t: string) => new RegExp(pxt.appTarget.uploadApiStringsBranchRx).test(t))

to make the linter happy

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

Successfully merging this pull request may close these issues.

None yet

3 participants