Skip to content

Port build tasks to Go script#2748

Merged
mislav merged 2 commits intotrunkfrom
makefile-rewrite
Jan 21, 2021
Merged

Port build tasks to Go script#2748
mislav merged 2 commits intotrunkfrom
makefile-rewrite

Conversation

@mislav
Copy link
Contributor

@mislav mislav commented Jan 8, 2021

This is to enable build tasks on Windows.

Fixes #2745

This is to enable build tasks on Windows.
Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

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

I am not super familiar with Make and all its functionality that we are trying to replicate so I will refrain from approving, but from a code standpoint this looks good to me. I left one question.


func sourceFilesLaterThan(t time.Time) bool {
foundLater := false
_ = filepath.Walk(".", func(path string, info os.FileInfo, err error) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work properly if the script is not run from the root directory? Is that a case we need to handle here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a good point! Originally, running make from a project directory other than root wouldn't work because there wouldn't be a Makefile there, but now that our build process is an ordinary Go script, it makes sense to add a guard that it must be run from a certain directory. I will add something like that.

@mislav
Copy link
Contributor Author

mislav commented Jan 13, 2021

I am not super familiar with Make and all its functionality that we are trying to replicate

Makefiles cam be arcane, but the basic functionality that I'm trying to replicate here is its mechanism of file dependencies. Consider this example:

bin/gh: src/foo.go src/bar.go
  touch bin/gh

This defines a task for "building" bin/gh (this concrete implementation only creates an empty file). Implicitly, this will run only if bin/gh doesn't exist, or if it exists but some of its dependent files (listed as src/foo.go src/bar.go) have a newer modification time than bin/gh (e.g. they might have been edited since the last build). This behavior of make generally speeds up development by skipping compilation where it's not necessary.

The idea of this PR is, if there are common build tasks that anyone developing on GitHub CLI can benefit from, regardless of platform, we should implement them in Go rather than via Makefile. On the other hand, some tasks only make sense to remain in Makefile, for example make install/uninstall only works on *nix platforms and not on Windows.

Finally, some of our old tasks are "internal" to our team, meaning that they're not useful to any outside contributors. Those tasks are: site, site-docs, site-bump. I didn't port them now because there is no immediate benefit to that, but we can consider doing so in the future.

Copy link

@PaulinaParangerHr PaulinaParangerHr left a comment

Choose a reason for hiding this comment

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

Super Ultrasoud
Fast flexibility Is needed these days....
Jumping Creativity...
Access priority ....
Experimental tasks for better
Future

Kind regards

Copy link
Contributor

@samcoe samcoe left a comment

Choose a reason for hiding this comment

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

@mislav Thanks for the additional context, given that information I am 👍 on this

@mislav mislav merged commit f89346f into trunk Jan 21, 2021
@mislav mislav deleted the makefile-rewrite branch January 21, 2021 16:36
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.

Port build scripts from Makefile to Go

3 participants