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 TinkerMail by TinkerHost Service #560

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

greenreader9
Copy link
Contributor

Adds Domain Connect support to upcoming TinkerMail service

Copy link

github-actions bot commented Oct 7, 2024

Linter OK:

Linter result for tinkerhost.net.tinkermail.json

@greenreader9 greenreader9 marked this pull request as draft October 7, 2024 01:00
@greenreader9
Copy link
Contributor Author

greenreader9 commented Oct 7, 2024

Changed to DRAFT as the template may be updated within the next 24 hours. This comment will be updated when ready for merge

Ready for merge

@greenreader9 greenreader9 marked this pull request as ready for review October 7, 2024 01:29
@kerolasa
Copy link
Collaborator

kerolasa commented Oct 7, 2024

Something is wrong with linter check. It did not notice file name is missing .json

Templates $ dc-template-linter -inplace tinkerhost.net.tinkermail 
2024-10-07T15:04:47+01:00 ERR code=DCTL1003 dctl_note="file name does not use required pattern" expected=tinkerhost.net.tinkermail.json template=tinkerhost.net.tinkermail

@greenreader9
Copy link
Contributor Author

Forgetting the JSON extension is a bit embarrassing, let me fix that right now

@greenreader9
Copy link
Contributor Author

Fixed @kerolasa

@pawel-kow
Copy link
Member

Forgetting the JSON extension is a bit embarrassing, let me fix that right now

I think there is an issue with the github action as it selects only json files so any other file won't hit the linter at all

      - name: Get all changed json files
        id: changed-json-files
        uses: tj-actions/changed-files@v42
        with:
          files: |
             **.json

I think here we'd need to either let all files to run through linter, even if these are not related (README) or to have some smarter solution with exclusion lists...
Alternatively we may leave it for manual review if any file is not json. This should be quite easy to notice...

@kerolasa
Copy link
Collaborator

kerolasa commented Oct 8, 2024

@greenreader9 the template looks good to me.

Meanwhile, I think the following change to github actions might work. Should we try @pawel-kow?

diff --git a/.github/workflows/dc-template-lint.yml b/.github/workflows/dc-template-lint.yml
index 5d85d7c..3fb3d65 100644
--- a/.github/workflows/dc-template-lint.yml
+++ b/.github/workflows/dc-template-lint.yml
@@ -35,7 +35,9 @@ jobs:
         uses: tj-actions/changed-files@v42
         with:
           files: |
-             **.json
+             '**',
+             '!README.md',
+             '!template.schema'
 
       - name: Print all changed json files
         if: steps.changed-json-files.outputs.any_changed == 'true'

Copy link
Member

@pawel-kow pawel-kow left a comment

Choose a reason for hiding this comment

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

lgtm

@pawel-kow pawel-kow added this pull request to the merge queue Oct 8, 2024
Merged via the queue into Domain-Connect:master with commit 2498174 Oct 8, 2024
2 checks passed
pawel-kow added a commit that referenced this pull request Oct 8, 2024
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.

3 participants