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

Add DLang workflow #74

Closed
wants to merge 1 commit into from
Closed

Add DLang workflow #74

wants to merge 1 commit into from

Conversation

ErnyTech
Copy link

@ErnyTech ErnyTech commented Aug 27, 2019

Add DLang workflow

D is a general-purpose programming language,
for more information: dlang.org

This patch adds a minimal workflow that allows
you to compile D projects that use DUB (the D package registry).

I used the D Compiler Installation
(https://github.com/marketplace/actions/d-compiler-installation) for setup the compiler for D.

The logo used (dlang.svg) was taken from here
https://github.com/dlang/dlang.org/blob/master/images/dlogo_2015.svg
and is available under the BSL-1.0 license (https://github.com/dlang/dlang.org/blob/master/LICENSE.txt)

ci/dlang.yml Outdated Show resolved Hide resolved
ci/dlang.yml Show resolved Hide resolved
@andre2007
Copy link

andre2007 commented Sep 8, 2019

@ErnyTech I justed checked the times. Using the container functionality in github actions seems to be rather slow ~44 seconds just for setting up the container. Also on the second try it doesn't get faster.
For comparison, by using the install.sh script instead, the runtime of the whole job is ~15 seconds.

We could leave it in this pull request but in the long run we should find something more performant.
(Also installing dmd.deb package is as slow as using the container https://github.com/CodeMyst/ghactions-test/blob/master/.github/workflows/blank.yml)

@andre2007
Copy link

@ErnyTech

Could you change the workflow to this one?
It is several times faster than the container solution.

name: DLang

on: [push]

jobs:
  build:

    runs-on: ubuntu-latest
    
    steps:
    - uses: actions/checkout@v1
    - uses: mihails-strasuns/setup-dlang@v0
    - name: Build
      run: dub build
    - name: Run tests
      run: dub test

@ErnyTech
Copy link
Author

Yes absolutely, I will do it by tomorrow

D is a general-purpose programming language,
for more information: dlang.org

This patch adds a minimal workflow that allows
you to compile D projects that use DUB (the D package registry).

I used the D Compiler Installation
(https://github.com/marketplace/actions/d-compiler-installation) for setup the compiler for D.

The logo used (dlang.svg) was taken from here
https://github.com/dlang/dlang.org/blob/master/images/dlogo_2015.svg
and is available under the BSL-1.0 license (https://github.com/dlang/dlang.org/blob/master/LICENSE.txt)

Signed-off-by: Ernesto Castellotti <erny.castell@gmail.com>
@ErnyTech
Copy link
Author

@andre2007 done!

@andre2007
Copy link

@ErnyTech Fantastic, thanks a lot

@andre2007
Copy link

Hi @mscoutermarsh, Hi @jeremyepling
could you give feedback on this pull request. Is anything missing?

Kind regards
André

@andymckay
Copy link
Contributor

andymckay commented Oct 1, 2019

👋 Sorry everyone for the delay on this. At this point we are working on adding some guidelines for this repository in this PR #156. One thing I note is that this uses an Action outside of the actions org and our first pass of guidelines suggests that we wouldn't allow that.

Your next question then is probably "how can we get dlang added to the actions" and I'm not sure on the answer to that right now.

So sorry, there might be some work to figure out what to do with this workflow. Thank you 🙇 for your contribution and we'll try and get there.

Copy link
Contributor

@Geod24 Geod24 left a comment

Choose a reason for hiding this comment

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

Some wording requirements and documentation fixes. Unfortunately this is still blocker on the fate of the Github action being used.

@@ -0,0 +1,16 @@
name: DLang

on: [push]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
on: [push]
on:
push:
branches: [ master ]
pull_request:
branches: [ master ]

d037e97

jobs:
build:

runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like other actions are doing this too. However nothing in this action prevents it to be run on all three platforms at the moment.

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- uses: actions/checkout@v1
- uses: actions/checkout@v2


steps:
- uses: actions/checkout@v1
- uses: mihails-strasuns/setup-dlang@v0.2.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- uses: mihails-strasuns/setup-dlang@v0.2.0
- uses: mihails-strasuns/setup-dlang@v0.5.0

Comment on lines +13 to +16
- name: Build
run: dub build
- name: Run tests
run: dub test
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- name: Build
run: dub build
- name: Run tests
run: dub test
- name: Build & Test
run: |
# Build the project, with its main file included, without unittests
dub build --compiler=$DC
# Build and run tests, as defined by `unittest` configuration
# In this mode, `mainSourceFile` is excluded and `version (unittest)` are included
# See https://dub.pm/package-format-json.html#configurations
dub test --compiler=$DC

We might also want to publish artifacts, but that's a bit more involved.

@@ -0,0 +1,6 @@
{
"name": "D Programming Language",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"name": "D Programming Language",
"name": "D",

@@ -0,0 +1,6 @@
{
"name": "D Programming Language",
"description": "Build, run and test a D Programming Language project with Dub.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"description": "Build, run and test a D Programming Language project with Dub.",
"description": "Build, run and test a D project with Dub.",

"name": "D Programming Language",
"description": "Build, run and test a D Programming Language project with Dub.",
"iconName": "dlang",
"categories": ["D Programming Language"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"categories": ["D Programming Language"]
"categories": [ "D" ]

I looked up other languages and none is using "Programming Language" or "lang" suffix

@Geod24
Copy link
Contributor

Geod24 commented Jun 11, 2020

@andymckay : Is there anything our community can do to help us make progress on this ?

I've personally been using the action referenced here for many months to great success.
It was recently tagged as v1, and transferred to a more official location, dlang-community/setup-dlang . There is an issue open by @ethomson (dlang-community/setup-dlang#13) for Github to take ownership on it, but it has been stalled for a few months.

I understand those are difficult times and you are probably extremely busy, so we'd really like to make this as simple as possible. Looking at other actions in this repository, there are plenty of places where external actions, images, or scripts are used:

I also listed scripts here, because curling a script and piping it to bash is no different than running an arbitrary action, security wise. If that is somehow preferred to an action, we also have a script to do that, which works on all 3 platforms (https://dlang.org/install.sh).

It would be greatly appreciated if, given the new, official location of the action, you could agree to move forward on this. In which case, I can raise a new PR with the review I left on April 16th fixed, unless @ErnyTech gets to it first.

@Geod24 Geod24 mentioned this pull request Jun 11, 2020
@Geod24
Copy link
Contributor

Geod24 commented Jun 11, 2020

After reviewing the contributions guidelines, it turns out this issue has been addressed, and third-party official actions are accepted as long as they satisfy certain criteria (have a disclaimer and use the hash). So I submitted #546, which also contain my feedback here (and is up to the new standards).

@andymckay
Copy link
Contributor

We merged #546 so I think we can close this one now. Sorry @ErnyTech it took us so long to get around to merging a D workflow, but thank you for your contribution 🙇🏾

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.

5 participants