Skip to content

Comments

let dinifile.readFromEnv()'s environment parameter be const#9618

Merged
wilzbach merged 1 commit intodlang:masterfrom
Herringway:const-readfromenv-p1
Apr 15, 2019
Merged

let dinifile.readFromEnv()'s environment parameter be const#9618
wilzbach merged 1 commit intodlang:masterfrom
Herringway:const-readfromenv-p1

Conversation

@Herringway
Copy link
Contributor

More const is nice, yes?

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @Herringway! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#9618"

@RazvanN7
Copy link
Contributor

Buildkite failure is unrelated.

@wilzbach
Copy link
Contributor

CC @CyberShadow. Did you change anything in are recently? It's failing now on Buildkite due to a missing file.

@wilzbach wilzbach merged commit c7834f3 into dlang:master Apr 15, 2019
@CyberShadow
Copy link
Member

CyberShadow commented Apr 15, 2019

Did you change anything in are recently?

Yes, I did. I changed the structure of dub.sdl and updated .travis.yml too. But, the tests pass on my machine and on Travis. It seems to be a problem with the script running the tests on BuildKite - maybe it doesn't know how to test subpackages?

@CyberShadow
Copy link
Member

CyberShadow commented Apr 15, 2019

@CyberShadow
Copy link
Member

CyberShadow commented Apr 15, 2019

I already excluded the previous Jenkins-based project tester by checking the environment:

https://github.com/cybershadow/ae/blob/f449f54293a6fe73e7d16b44c61069cdbf02d3b2/sys/net/test.d#L76-L79

I've updated it to check for BuildKite:

https://github.com/CyberShadow/ae/releases/tag/v0.0.2389

Hopefully that will work.

auto sv = environment.lookup(name, len);
const sv = environment.lookup(name, len);
if (sv && sv.ptrvalue)
return cast(const(char)*)sv.ptrvalue; // get cached value
Copy link
Member

Choose a reason for hiding this comment

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

Why was this cast removed ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants