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.

Sign up for GitHub

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

Jump to bottom

chore: Cleanup CI #1117

Merged
merged 72 commits into from
Aug 6, 2024
Merged

chore: Cleanup CI #1117

merged 72 commits into from
Aug 6, 2024

Conversation

AlejandroCabeza
Copy link
Collaborator

@AlejandroCabeza AlejandroCabeza commented Jun 5, 2024 •
edited
Loading

Changes

  • Naming:
    • Standardise workflow names
    • Standardise and clean variables', jobs' and steps' names
  • Split some jobs into steps
  • Update actions vertsions
  • Logic
    • Change memory management variable handling
  • Add a couple clarifying comments

Tests

All tests pass. Except:

  • interop: hole punching
  • daily_devel: all (mac, windows and linux on amd64 with nim-devel)
  • daily_i386: devel (linux i386 nim-devel)

All other daily tests pass, I removed them from the PR in order for it to be merge-ready.

@kaiserd
Copy link
Collaborator

kaiserd commented Jun 6, 2024

Thank you 🙌
Can you please elaborate a bit on the proposed changes?

@AlejandroCabeza
Copy link
Collaborator Author

Thank you 🙌 Can you please elaborate a bit on the proposed changes?

@kaiserd Still working on it, but the idea is threefold: Understand precisely how the workflows behave (looking to properly implement the maxver and minver ones we talked about months ago), reduce duplicated workflows, and improve naming and descriptions (so everyone can understand them better).

@AlejandroCabeza AlejandroCabeza force-pushed the feature/cleanup-ci branch 2 times, most recently from f04656d to aac51f4 Compare July 10, 2024 19:15
@AlejandroCabeza AlejandroCabeza marked this pull request as ready for review July 11, 2024 08:36
@kaiserd
Copy link
Collaborator

kaiserd commented Jul 17, 2024

What is the status here? Is is the PR ready for re-review?

@AlejandroCabeza
Copy link
Collaborator Author

What is the status here? Is is the PR ready for re-review?

I've fixed answered threads but I'm waiting for answers on a couple or three others.

kaiserd
kaiserd reviewed Jul 26, 2024
View reviewed changes
Copy link
Collaborator

@kaiserd kaiserd left a comment

Choose a reason for hiding this comment

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

LGMT. (reviewed in pp session)
(still needs tests to be fixed)

@AlejandroCabeza AlejandroCabeza force-pushed the feature/cleanup-ci branch 10 times, most recently from 8f5855a to c5a3632 Compare July 31, 2024 10:49
@AlejandroCabeza
Merge branch 'master' into feature/cleanup-ci
183c6a6 
* master:
  feat: adding onValidated observer (#1128)
  fix: add gcc 14 support (#1151)
  fix(ci): windows-amd64 (Nim version-1-6) (#1160)
@AlejandroCabeza
Fix wrong keys.
e9b3d24
@AlejandroCabeza AlejandroCabeza enabled auto-merge (squash) August 5, 2024 10:06
lchenut
lchenut approved these changes Aug 5, 2024
View reviewed changes
Copy link
Contributor

@lchenut lchenut left a comment

Choose a reason for hiding this comment

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

LGTM

@AlejandroCabeza AlejandroCabeza merged commit 6ec038d into master Aug 6, 2024
14 checks passed
@AlejandroCabeza AlejandroCabeza deleted the feature/cleanup-ci branch August 6, 2024 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jm-clius jm-clius jm-clius left review comments

@diegomrsantos diegomrsantos diegomrsantos left review comments

@arnetheduck arnetheduck arnetheduck left review comments

@lchenut lchenut lchenut approved these changes

@kaiserd kaiserd kaiserd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
nim-libp2p
Status: done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@AlejandroCabeza @kaiserd @arnetheduck @diegomrsantos @lchenut @jm-clius