magefile / mage

a Make/rake-like dev tool using Go
https://magefile.org
Apache License 2.0
4.01k stars 250 forks source link

Enhancement: support putting magefiles in a subdirectory #395

Closed natefinch closed 2 years ago

natefinch commented 2 years ago

Describe the feature

I propose that we add support for a ./magefile subdirectory that could contain all your magefiles without using the mage build tag and without putting the files in package main. Then if you run mage in that directory or the one above it, mage would use the files in that directory as the files to build into the compiled binary.

What problem does this feature address?

The major benefit of this is mostly that we stop confusing the language server. Currently, when you write magefiles, you give them the mage build tag, and put them in package main.

The problem is that the language server won't consider them when it's running in your editor, because the build tag isn't specified. If you DO specify the build tag, then you sometimes will have two different packages in the same directory (whatever package normally exists in the root directory, and then package main for the magefiles).

If you happen to not have other go code in the root directory or that code is also package main, then it's ok. But if it's not, the language server will barf, because you can't have two packages in the same folder.

Even if you don't have other go code, the language server still will tend to barf, because you have package main and no func main(){} defined.

The other benefit is that sometimes it's just nice to have your build/dev scripts squirreled away in a subdirectory, rather that cluttering up your root directory.

mgabeler-lee-6rs commented 2 years ago

I would propose also considering ./.magefile/ just to allow things to be "prettier"

natefinch commented 2 years ago

Yeah, I don't generally like putting code in a hidden folder. I feel like .folders should mainly be for configuration, not for code you'll want to edit and read fairly often. This is doubly true because on some OSes, it can be annoying to view/navigate to hidden folders. That doesn't mean it couldn't be opt-in, I guess.

mgabeler-lee-6rs commented 2 years ago

Yeah, I'm torn on that. magefiles are, to me, an interesting hybrid. in most cases, build configs are just that -- configs. But magefiles, while fitting that function, are also code.

Personally I'd be fine with an env var used to set the (relative) path(s) to look for the magefile. I already use a small wrapper script in my repos for running mage, so adding it to that would be zero effort.

mirogta commented 2 years ago

This new feature would be a nice improvement to our builds - we already put mage files into a subdirectory (not prefixed with a dot).

Michael-F-Ellis commented 2 years ago

This makes a lot of sense. FWIW, I prefer ./magefiles to ./.magefiles.

Question: I've got large project with magefiles in subdirectories and a top level magefile that cd's to the subdirectories to run them. Would that still work with the proposed scheme?

natefinch commented 2 years ago

Depends a lot on what exactly you're doing and how we'd implement this... but my assumption is that mage would run in the current directory as it always has, so anything relative would be relative to where you run mage. So.... hopefully?

Definitely we would try to make it backwards compatible with how people are working today... but given that it's code that can do anything, there's always a chance something won't work right.

I'm going to release this as a beta feature first, before releasing it into the wild, so everyone has a chance to try it out before it goes live.

sheldonhull commented 2 years ago

I'm already doing this in one of my projects example here following the tips and docs. Is this focused on not requiring any file that imports from the subdirectory?

I typically include the zero install file + import file in the root so I can easily use in CI, and put all my tasks in 1 or more directories like magefiles/proj, magefiles/docker etc.

mirogta commented 2 years ago

@sheldonhull my understanding was that the "magefiles" (i.e. files with //go:build mage tag) in the subdirectory wouldn't have the //go:build ignore or //go:build mage or // +build mage build tags/comments, which would make the source code better discoverable via e.g. gopls (Go language server) and therefore easier to work with in e.g. VSCode.

In your example, your "magefile" is the magefile.go which is still in the root.

sheldonhull commented 2 years ago

Very cool! @natefinch @mirogta one last suggestion that you might consider.

I tried a couple different variations and ended up using .magefiles with prefix dot.

The default folder being that way would ensure adding Mage files doesn't impact the code coverage or other test automation. So ignores underscore and Dot prefixed directories by default. This may seem minor but I noticed that my code coverage got impacted pretty heavily as my Mage files got larger and writing tests for mage files isn't quite/probably lower priority than app code.

So far this works really well as the default. YMMV!

natefinch commented 2 years ago

Yeah, the problem I have with dot folders is that they get hidden in some OS configurations, and that's not good for directories that hold code you're supposed to be able to change whenever you want. But I guess we could also just check for both. That wouldn't be any harder.

On Sat, Mar 12, 2022, 3:26 PM sheldonhull @.***> wrote:

Very cool! @natefinch https://github.com/natefinch one last suggestion that you might consider.

I tried a couple different variations and ended up using .magefiles with prefix dot.

The default folder being that way would ensure adding Mage files doesn't impact the code coverage or other test automation. So ignores underscore and Dot prefixed directories by default. This may seem minor but I noticed that my code coverage got impacted pretty heavily as my Mage files got larger and writing tests for mage files isn't quite/probably lower priority than app code.

So far this works really well as the default. YMMV!

— Reply to this email directly, view it on GitHub https://github.com/magefile/mage/issues/395#issuecomment-1065957151, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAYJZSFJRTCNDBI6F6T3WJTU7T4W5ANCNFSM5KGZ3AWQ . You are receiving this because you were mentioned.Message ID: @.***>

mirogta commented 2 years ago

I'd also prefer checking both.

natefinch commented 2 years ago

I think we forgot to add code to check for .magefile ... PRs welcome :)