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

Automate copyright checks (9882 companion) #401

Merged
merged 8 commits into from
Apr 19, 2024

Conversation

tibetiroka
Copy link
Member

@tibetiroka tibetiroka commented Mar 17, 2024

Adds the copyright CI checks from ES, though they now run unconditionally since pretty much everything should be editing the copyright file.

The only issue is that now the workflow file would probably be included in the plugin, which seems unfortunate. I think it can be solved by uploading custom artifacts to future releases, but we have to make sure it doesn't break the continuous nor the release builds.

With the included CD stuff we should be able to generate the correct artifacts, but the plugin manifest in https://github.com/endless-sky/endless-sky-plugins will likely have to be updated.

@tibetiroka tibetiroka marked this pull request as ready for review March 17, 2024 07:41
@tibetiroka
Copy link
Member Author

I can't get the jobs to start. Is this a permission thing?

Copy link
Member

@quyykk quyykk left a comment

Choose a reason for hiding this comment

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

I'm not sure why it doesn't run. Maybe it's not enabled in the repo settings? @Zitchas

Or maybe it's just a security thing since these workflow files are new.

@@ -0,0 +1,38 @@
name: CD
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a CD workflow?

Copy link
Member Author

Choose a reason for hiding this comment

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

We need the CD if we don't want to include the .github files in the release I think?

@@ -0,0 +1,32 @@
name: Release CD
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need a release CD workflow?

Copy link
Member Author

@tibetiroka tibetiroka Mar 17, 2024

Choose a reason for hiding this comment

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

This could probably be folded into the standard CD workflow, I just don't have enough experience and confidence so just copied things over from ES. File names might have to be different, idk.

@warp-core
Copy link
Contributor

warp-core commented Mar 17, 2024

GitHub just won't run workflows added in a PR if the file doesn't exist in the base branch.
Actually, no, it won't run them if the file doesn't exist in the repository. If, for example, your PR head was a different branch in the same repository, it would run the workflows.

@warp-core
Copy link
Contributor

warp-core commented Apr 15, 2024

I think we can have GitHub not include the workflow files in the source code archive with a .gitattributes file.
endless-sky/endless-sky does this:

# Don't export certain files via `git archive` (e.g. via "Download ZIP")
.gitattributes export-ignore
.gitignore export-ignore
.gitmodules export-ignore
.github export-ignore

Which means we don't need to make a continuous release.

Actually, we don't need the CD at all if we do that. The CD would just produce an archive with the same contents, but it'd be taking up space and would expire. The source code archives don't count towards a limited storage capacity.

@tibetiroka
Copy link
Member Author

If that works, that sounds like a great solution.

@warp-core warp-core merged commit 790e9fa into endless-sky:master Apr 19, 2024
1 check passed
@tibetiroka tibetiroka deleted the copyright-ci branch April 19, 2024 18:10
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.

3 participants