Skip to content

[CI] Run hadolint#2456

Merged
Frenzie merged 3 commits intoFreshRSS:devfrom
SuperSandro2000:sandro-hadolint
Jul 23, 2019
Merged

[CI] Run hadolint#2456
Frenzie merged 3 commits intoFreshRSS:devfrom
SuperSandro2000:sandro-hadolint

Conversation

@SuperSandro2000
Copy link
Contributor

@SuperSandro2000 SuperSandro2000 commented Jul 22, 2019

As requested here

I don't like that it needs jq but I am to lazy to write that json select with bash. It is also probably more robust and easier to understand this way.

This would also require to add a GItHub Token under $GITHUB_TOKEN in Travis. If we leave this out the CI periodically fails cause GitHub API gets rate limited quite easily.
The URL would then change to:
https://api.github.com/repos/hadolint/hadolint/releases/latest?access_token="$GITHUB_TOKEN"

I am a bit unsure where to put it or if it should be blocking for the other CI jobs. Let me know if I should move it to another section.

@Frenzie
Copy link
Member

Frenzie commented Jul 22, 2019

Could you please also fix the issues it points out?

Docker/Dockerfile:55 DL3025 Use arguments JSON notation for CMD and ENTRYPOINT arguments
Docker/Dockerfile-Alpine:51 DL3025 Use arguments JSON notation for CMD and ENTRYPOINT arguments
Docker/Dockerfile-QEMU-ARM:67 DL3025 Use arguments JSON notation for CMD and ENTRYPOINT arguments

Regarding placement, if you test for something like [ -z ${HADOLINT+x} ]; you can do something similar to the VALIDATE_STANDARD/CHECK_TRANSLATION stuff and combine it with the shellchecks script. Or it's probably better to also add something like LINT=yes for clarity.

@Alkarex Alkarex requested a review from Frenzie July 22, 2019 18:09
@SuperSandro2000
Copy link
Contributor Author

SuperSandro2000 commented Jul 22, 2019

I changed it to only run in one job cause everything else is kinda pointless. The Dockerfiles don't change with the php version.

About fixing that issue: It would require us to move the CMD to it's own file cause I don't know a way to use ( with the json notation.

Or we just ignore it for now.

How do we wan't to handle the GitHub Token? I don't really want to hardcode one.

@Frenzie
Copy link
Member

Frenzie commented Jul 22, 2019

Or we just ignore it for now.

Yes, in that case it's probably clearest to stick a # hadolint ignore=DL3025 above the relevant lines.

@Frenzie
Copy link
Member

Frenzie commented Jul 22, 2019

@SuperSandro2000 I added a GITHUB_TOKEN variable to Travis with no scopes. Is that correct?

@SuperSandro2000
Copy link
Contributor Author

@SuperSandro2000 I added a GITHUB_TOKEN variable to Travis with no scopes. Is that correct?

Yes, as we only use it for the API we don't need any special permission. Thanks.

@SuperSandro2000
Copy link
Contributor Author

This should be done.

@Frenzie Frenzie merged commit 4e0acf5 into FreshRSS:dev Jul 23, 2019
@Frenzie
Copy link
Member

Frenzie commented Jul 23, 2019

Awesome, thanks!

@Alkarex Alkarex added this to the 1.14.3 milestone Jul 23, 2019
Alkarex added a commit that referenced this pull request Jul 23, 2019
#2454
#2455
#2456
+hadolint remove PIP warning
@SuperSandro2000 SuperSandro2000 deleted the sandro-hadolint branch July 23, 2019 08:13
mdemoss pushed a commit to mdemoss/FreshRSS that referenced this pull request Mar 25, 2021
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