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

ci, cd: merge #2689

Open
wants to merge 6 commits into
base: development
Choose a base branch
from
Open

Conversation

div72
Copy link
Member

@div72 div72 commented Jun 1, 2023

This PR changes the CI to upload the created artifacts and removes the cd system. The created artifacts can be downloaded from the bottom of the job page. (See: https://github.com/div72/Gridcoin-Research/actions/runs/5085626106 for an example)

Upsides:

  1. Since the CD is only used when making new releases, it's not tested as much as the CI. This can lead to something breaking at release time which makes it a PITA. Essentially removes the maintenance burden of the CD.
  2. Since the artifacts are always uploaded, this helps people test PRs without having to compile them.

Downsides:

  1. Only GitHub users can download artifacts. This is not a dealbreaker but reduces upside 2.
  2. Artifacts are only visible once the whole CI run ends.

This PR also changes the output directories for some release files. See the relevant commit messages for the rationale.

div72 added 6 commits May 26, 2023 01:22
Rationale:
    This will make testing PRs easier for other people as the builds
    will always be present but the main reason for this change is to
    merge the CI and CD.
Rationale:
    Since the CI uploads artifacts now, there's no need for a separate
    CD.
Rationale:
    So that installer packages are also included in the artifacts.
Rationale:
    The @abs_top_srcdir@ for CHANGELOG.md in share/setup.nsi.in points
    to the build dir when building in a distdir.
Rationale:
    Moving the installer is not optimal as Make expects the file to be
    in the specified path of the rule in order to decide on whether to
    re-create the file or not. This also breaks the CI's artifact code
    as it expects the file to be present in the same way.
Rationale:
    Same as the previous commit.
@div72 div72 requested a review from jamescowens June 1, 2023 19:26
@div72 div72 self-assigned this Jun 1, 2023
@div72 div72 marked this pull request as ready for review June 1, 2023 19:29
export GRIDCOIN_CONFIG="--enable-reduce-exports"
export DEP_OPTS="NO_QT=1"
Copy link
Member

Choose a reason for hiding this comment

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

is there a reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

The artifacts should contain the Qt builds as CI is overloaded to be used for releases?

@jamescowens
Copy link
Member

Does this need to be tweaked now that we merged the cmake PR?

@div72
Copy link
Member Author

div72 commented Aug 23, 2023

No, unless we want to expose artifacts from the CMake CI as well. This PR is mainly done to reduce the maintenance burden of the CD for which the artifacts from the CI are enough.

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