Skip to content

Conversation

@ichizok
Copy link
Member

@ichizok ichizok commented Dec 19, 2020

  • Reinstalling gettext, installing packages, smoketests and updating vimtags are need only for the build for publish.
  • Remove the envvars CONFOPT and LANGOPT, and set them explicitly in configure step.
  • Separate executing xcode-install into "Set up Xcode" step, the condition matrix.xcode is set.

@ichizok ichizok marked this pull request as draft December 19, 2020 12:17
@ichizok ichizok marked this pull request as ready for review December 19, 2020 13:22
@ychin
Copy link
Member

ychin commented Dec 19, 2020

I'm a little hesitant to move installing gettext, packages, scripting languages, and smoketest to a publish only step. In my mind, those are useful regression tests in addition to just being part of publishing the app (and a lot of them were added to address real specific past issues that we have seen).

For example, if I'm working on MacVim's localization, I could inadvertently broke the message translation support (basically the work done in #1070), and I would prefer if I know about it immediately instead of the next publish which could be months later. Same thing for scripting language support. I would prefer to know if it broke immediately. There are also people who syncs to HEAD instead of following our release schedule as well.

Also, all the steps that we are omitting here takes half a minute to complete (if gettext is cached, which will always be the case unless we get an updated version, or we don't run CI for 7 days). They seem like pretty good bang for the buck compared to the Vim tests which take up the bulk of the time which I found to frequently catch issues with the tests themselves or upstream Vim issues more than MacVim itself.

@ichizok
Copy link
Member Author

ichizok commented Dec 19, 2020

Reinstalling gettext, installing packages, smoketests and updating vimtags are need only for the build for publish.

My writing might have confused you; "the build for publish" means the build whose matrix.publish set to true, not "build only when publishing". That is, the above processes run on one job at every push and pull-request event.
However, maybe they should run on all jobs?

@ychin
Copy link
Member

ychin commented Dec 19, 2020

Oh actually you are right. I think I confused myself when I read your diff 😅.

I think I'm fine with the PR as-is.

@ychin ychin merged commit 08d4327 into macvim-dev:master Dec 20, 2020
@ichizok ichizok deleted the fix/ci branch December 20, 2020 00:35
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.

2 participants