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

Add macOS nightlies to CI #464

Merged
merged 1 commit into from
Feb 20, 2021
Merged

Add macOS nightlies to CI #464

merged 1 commit into from
Feb 20, 2021

Conversation

Sija
Copy link
Contributor

@Sija Sija commented Jan 14, 2021

Refactored some parts as well like include-ing windows in the matrix instead of having it there from start - especially apt as windows support is still in-the-works.

@Sija
Copy link
Contributor Author

Sija commented Jan 21, 2021

This could be merged...

@Sija
Copy link
Contributor Author

Sija commented Jan 30, 2021

@bcardiff Could you please review this?

@bcardiff
Copy link
Member

Is there a reason to exclude windows from using crystal latest?

@Sija
Copy link
Contributor Author

Sija commented Feb 19, 2021

@bcardiff yes, it doesn't work. for more detailed explanation please ask @oprypin.

@oprypin
Copy link
Member

oprypin commented Feb 19, 2021

And the reason for that is crystal-lang/distribution-scripts#64 (comment)

@oprypin
Copy link
Member

oprypin commented Feb 19, 2021

@Sija Why do you have to change the whole file's formatting to adhere to your liking?

@Sija
Copy link
Contributor Author

Sija commented Feb 19, 2021

@oprypin Whole file? Don't exaggerate, adding some spaces for readability and changing name of one job is not whole file. What's wrong with those changes anyway?

@asterite
Copy link
Member

asterite commented Feb 19, 2021

@Sija They are irrelevant to the change you want to make.

If you want to change someone else's style, I think that should be discussed and done separately.

Code is written by people if you go and change that when it's not needed, while also adding additional changes, the ones that wrote the original code might think "What's your problem with my style? BAM. I don't like your PR."

Not that that will effectively happen. But at least in my case when I contribute to a project I tend to avoid changing style things if they have nothing to do with the PR's goal.

@asterite
Copy link
Member

They also make reviewing the PR harder! "Why was this changed? Is this relevant? I don't understand."

Copy link
Member

@bcardiff bcardiff 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 fine the changes. Let's keep it calm. But I agree that changes that satisfies author's taste should be avoided in general.

@bcardiff bcardiff merged commit 1bc1ae0 into crystal-lang:master Feb 20, 2021
@bcardiff bcardiff added this to the v0.14.0 milestone Feb 20, 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.

None yet

5 participants