Skip to content

Conversation

@GNMoseke
Copy link

Depends on #1

This adds basic CI (linting and tests on macOS/ubuntu) for PRs. Some things to keep in mind:

  • the .swift-format file here is opinionated based on my personal preferences, I am happy to tweak them based on other maintainers' thoughts
  • By default I've set swift to build with -Werror as I think it's a good practice to get into off the starting block. Again, if we forsee unavoidable errors (@deprecated() 😡 ) being added, happy to remove those flags
  • The macOS tests are running only against the latest stable Xcode - happy to dig into more custom options there if desired

@GNMoseke GNMoseke force-pushed the feat/ci branch 2 times, most recently from 1371c7e to ce9b64e Compare November 21, 2024 15:45
Copy link
Collaborator

@JaapWijnen JaapWijnen left a comment

Choose a reason for hiding this comment

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

Looks good. I tend to agree to those initial formatting rules. Fine to change some if we ever run into anything particular of course.

Might have to disable the macOS tests for now since they will either make the job fail or won't do anything since everything is commented out by #ifdef canImport(_Differentiation)

@JaapWijnen
Copy link
Collaborator

Also let's copy this config over to the other repos once we have agreed on these and this is merged

Copy link

@tmcdonell tmcdonell left a comment

Choose a reason for hiding this comment

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

  • I don't have enough experience writing swift to be opinionated on style (except no hard tabs, ever), so happy to go with your recommendations
  • I agreed having -Werror from the start is a good idea
  • We should be able the make a template repository so we don't have to manually copy and paste to each new repo

@GNMoseke GNMoseke merged commit 34370a1 into main Nov 22, 2024
4 checks passed
@GNMoseke GNMoseke deleted the feat/ci branch November 22, 2024 11:25
@JaapWijnen
Copy link
Collaborator

JaapWijnen commented Nov 28, 2024

I'd like to add this rule in a next PR any thoughts @GNMoseke?
NoAccessLevelOnExtensionDeclaration I really dislike not being able to read the access level at the location of the definition.
How do we want to keep these settings in sync across repos? We already have 4 repos where formatting is relevant atm.

  • swift-differentiation
  • swift-differentiation-testing (in a sub folder)
  • swift-numerics-differentaible
  • swift-collections-differentiable

Or do we do this by hand for now?

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.

5 participants