Skip to content

boards/msba2: Do not set PORT.#10454

Merged
miri64 merged 1 commit intoRIOT-OS:masterfrom
jcarrano:msba2-fix-port
Nov 28, 2018
Merged

boards/msba2: Do not set PORT.#10454
miri64 merged 1 commit intoRIOT-OS:masterfrom
jcarrano:msba2-fix-port

Conversation

@jcarrano
Copy link
Copy Markdown
Contributor

@jcarrano jcarrano commented Nov 22, 2018 •
edited
Loading

Contribution description

Boards should not set PORT and should not have code conditional on PORT as that causes PORT to be evaluated and the build to fail even if this varible is not needed.

Exporting has the same effect.

This fixes the MSBA2 board by declaring PORT_LINUX and PORT_DARWIN instead. PORT_DARWIN is obviously wrong, but it is broken with or without this PR.

Testing procedure

CI should test that this builds.

I tested make term in a MSBA2. make flash was not tested (I don't have the program).

Issues/PRs references

Needed by #10342 so it does not fail.
See also #10440 .

@jcarrano jcarrano added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: build system Area: Build system Area: boards Area: Board ports labels Nov 22, 2018
@jcarrano
Copy link
Copy Markdown
Contributor Author

jcarrano commented Nov 22, 2018

@kYc0o, @smlng Somebody with a mac and a msba2 can tell me the correct port?

@jcarrano jcarrano added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 22, 2018
@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 22, 2018

CI should test this for you.

How? There are no msba2 connected to the CI.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 22, 2018

@emmanuelsearch has some msba2 and a Mac :-)

@jcarrano
Copy link
Copy Markdown
Contributor Author

jcarrano commented Nov 22, 2018

here are no msba2 connected to the CI.

d'oh. I'm editing the description.

@miri64
Copy link
Copy Markdown
Member

miri64 commented Nov 22, 2018

@emmanuelsearch has some msba2 and a Mac :-)

Correction: He knows where the msba2 are ;-).

@jcarrano jcarrano mentioned this pull request Nov 22, 2018
Closed
@jcarrano jcarrano added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Nov 26, 2018
miri64
miri64 reviewed Nov 27, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

Tested by flashing and term on msba2.

@miri64 miri64 added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines labels Nov 27, 2018
@jcarrano jcarrano removed the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 28, 2018
@jcarrano
Copy link
Copy Markdown
Contributor Author

jcarrano commented Nov 28, 2018

Done.

@jcarrano jcarrano added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Nov 28, 2018
@jcarrano
boards/msba2: Do not set PORT.
c4ccaec 
Boards should not set PORT and should not have code conditional on
PORT as that causes PORT to be evaluated and the build to fail even
if this varible is not needed.

Exporting has the same effect.

This fixes the MSBA2 board by declaring PORT_LINUX and PORT_DARWIN
instead.
miri64
miri64 approved these changes Nov 28, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@miri64 miri64 left a comment

Choose a reason for hiding this comment

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

ACK

@miri64 miri64 merged commit 4b06d64 into RIOT-OS:master Nov 28, 2018
@aabadie aabadie added this to the Release 2019.01 milestone Dec 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miri64 miri64 miri64 approved these changes

@emmanuelsearch emmanuelsearch Awaiting requested review from emmanuelsearch

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: build system Area: Build system CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Reviewed: 5-documentation The documentation details of the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

Release 2019.01

Development

Successfully merging this pull request may close these issues.

3 participants

@jcarrano @miri64 @aabadie