Skip to content

Add MacPorts support to the build system #10736

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

Merged
merged 3 commits into from
Oct 9, 2019
Merged

Add MacPorts support to the build system #10736

merged 3 commits into from
Oct 9, 2019

Conversation

Lucius-Q-User
Copy link
Contributor

@Lucius-Q-User Lucius-Q-User commented Oct 8, 2019

PR Summary

Add MacPorts support to the build system

PR Context

MacPorts is one of the "Missing package managers for OS X".
The original build scripts only supported Homebrew and it is inadvised to have
both installed on a same system.
This change makes the build script use whatever package manager is currently
installed, or direct the user to download one of them if neither is present.
The build documentation has been updated to reflect that another package
manager is now also supported.

PR Checklist

MacPorts is one of the "Missing package managers for OS X".
The original build scripts only supported Homebrew and it is inadvised to have
both installed on a same system.
This change makes the build script use whatever package manager is currently
installed, or direct the user to download one of them if neither is present.
@msftclas
Copy link

msftclas commented Oct 8, 2019

CLA assistant check
All CLA requirements met.

Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

See comments

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 8, 2019
@TravisEz13 TravisEz13 self-assigned this Oct 8, 2019
@ghost ghost removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 8, 2019
@TravisEz13
Copy link
Member

There is a spelling error.

Here is my function to test spelling.

function Test-Spelling
{
    param(
        [string[]]
        $Paths,
        [switch]
        $Fix

    )

    if(!(Get-Command mdspell -ErrorAction SilentlyContinue))
    {
        brew install yarn
        sudo yarn global add 'markdown-spellcheck@0.11.0'
    }

    $fileList = @()

    foreach($path in $Paths)
    {
        if($path -match '^\.[/\\]')
        {
            $fileList += ($path -replace '^\.[/\\]')
        }
        else {
            $fileList += $path
        }
    }

    $extraParams = @()

    if(!$Fix.IsPresent)
    {
        $extraParams += '--report'
    }

    Write-Verbose "Testing spelling for $fileList" -Verbose
    mdspell $fileList --ignore-numbers --ignore-acronyms @extraParams --en-us --no-suggestions
}

@Lucius-Q-User
Copy link
Contributor Author

Lucius-Q-User commented Oct 8, 2019

There is a spelling error.

The script only complains about the words "2.x" (referring to the required .net core version) and "MacPorts". Looks like false positives to me.

@TravisEz13
Copy link
Member

TravisEz13 commented Oct 8, 2019

Spelling errors must be fixed for the PR to be accepted. Please run the tool and add the words to the dictionary. There is a fix switch in my script which will prompt you to add it to the dictionary

@TravisEz13 TravisEz13 added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Oct 8, 2019
@TravisEz13 TravisEz13 added this to the 7.0.0-preview.5 milestone Oct 8, 2019
@TravisEz13
Copy link
Member

There was a random failure verifying a URL. I retried that task.

@TravisEz13
Copy link
Member

@PoshChan Please remind me in 1 hour

Copy link
Member

@TravisEz13 TravisEz13 left a comment

Choose a reason for hiding this comment

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

Hold for compliance review

@ghost ghost added the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 8, 2019
@TravisEz13 TravisEz13 removed the Waiting on Author The PR was reviewed and requires changes or comments from the author before being accept label Oct 8, 2019
@TravisEz13
Copy link
Member

@Lucius-Q-User To explain what triggered the review, MacPorts license is not clearly declared on GitHub (image below). So, I'm waiting on a manual review of the use of this software in our project. They don't give me a specific ETA.

MacPro License

image

MIT License

image

@PoshChan
Copy link
Collaborator

PoshChan commented Oct 8, 2019

@TravisEz13, this is the reminder you requested 1 hour ago

@TravisEz13
Copy link
Member

@Lucius-Q-User I'm monitoring the compliance review and I'd expect an answer within a week at this point. Ping me if we don't get an answer by that time.

@TravisEz13 TravisEz13 changed the title Add MacPorts support to the build system Add MacPorts support to the build system Oct 9, 2019
@TravisEz13 TravisEz13 merged commit 273b4ab into PowerShell:master Oct 9, 2019
@ghost
Copy link

ghost commented Oct 23, 2019

🎉v7.0.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants