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

build(task-runner): add self-documented task runner #486

Merged
merged 4 commits into from
Apr 12, 2020
Merged

Conversation

DrSensor
Copy link
Contributor

@DrSensor DrSensor commented Mar 4, 2020

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • New Binding Issue #___
  • Code style update
  • Refactor
  • Build-related changes
  • Other, please describe: developer experience for contributor or maintainer

Does this PR introduce a breaking change? (check one)

  • Yes. Issue #___
  • No

The PR fulfills these requirements:

  • It's submitted to the dev branch and not the master branch
  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix: #xxx[,#xxx], where "xxx" is the issue number)

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature (to avoid wasting your time, it's best to open a suggestion issue first and wait for approval before working on it)

Other information:
As a new/first contributor, I have difficulty to check the small changes that I made is correct, especially when I want to check if it works on the examples repo. I'm choosing mask because it's markdown which can be used as some sort of recipe. (also I'm more familiar with mask codebase)
tauri-mask-run-example

DrSensor added 3 commits March 4, 2020 00:55
`mask` can also act as self-documented task runner
https://github.com/jakedeichert/mask

TODO: add demo.gif on opening this PR
with assumption it only happen on examples/rust
@DrSensor DrSensor marked this pull request as ready for review March 4, 2020 09:48
@DrSensor DrSensor requested a review from a team as a code owner March 4, 2020 09:48
Copy link
Member

@nothingismagick nothingismagick left a comment

Choose a reason for hiding this comment

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

This is awesome.

@lucasfernog
Copy link
Member

Wow this is so cool.

Btw do we have a contribution guide?

@nothingismagick
Copy link
Member

@jbolda
Copy link
Member

jbolda commented Mar 4, 2020

@DrSensor very cool! I have been working on a good workflow using yarn workspaces for the examples, but that won't cover the rust based examples. This looks like it has the capability though which is great. It appears that this may not work on Windows though which is unfortunate 😣.

My other thought is that I wonder if this is best added to the examples repo?

@DrSensor
Copy link
Contributor Author

DrSensor commented Mar 4, 2020

My other thought is that I wonder if this is best added to the examples repo?

@jbolda That's possible and it will be composable by referencing ./examples/maskfile.md in some of ./maskfile.md command.
But I don't know if that's a good idea. I think each example in the examples repo should be separated as a git-submodule and converted into template repo first. Early adopters might only interested in cloning then run a specific example rather than cloning the whole examples.

It appears that this may not work on Windows though which is unfortunate 😣.

I'm not a Windows user but this task-runner might work on Windows via WSL. The thing that I'm not sure is if WSL handle the path in the same way as Linux 🤔 (cc: @tensor-programming )

@jbolda
Copy link
Member

jbolda commented Mar 5, 2020

I was actually thinking only have the maskfile in https://github.com/tauri-apps/examples as the functions relate specifically to them. I guess my personal opinion regarding the examples is that they serve strictly as test grounds and the expectation is that they won't be cloned to start a project. Instead we push towards using the CLI to start a project. What do others think? It seems to always be an ongoing discussion.

The paths certainly could be an issue on Windows, but if it works in bash and WSL that's better than nothing.

@nothingismagick
Copy link
Member

@jbolda - can't we do both?

@jbolda
Copy link
Member

jbolda commented Mar 5, 2020

@nothingismagick If that is what you would like to do, it's not unreasonable. We would have to maintain it in both places, but there are worse situations.

@tensor-programming
Copy link
Member

tensor-programming commented Mar 5, 2020

Ill look into making it work on windows but bash and wsl only support is a bit unacceptable. Most windows devs don't use either or would rather not use either.

Mask looks like it works out of the box for Powershell. Just put some powershell in a block and it works without any changes to the codebase. I added a block to the project's main build block:

~~~Powershell
cargo build --release
~~~

@nothingismagick
Copy link
Member

@tensor-programming - with your powershell amendment this could be approved?

@tensor-programming
Copy link
Member

its being looked at here: jacobdeichert/mask#45

They said they would review it next week.

@lucasfernog
Copy link
Member

can this be merged already?

@jbolda
Copy link
Member

jbolda commented Mar 29, 2020

We were waiting on the upstream next version with @tensor-programming changes.

@tensor-programming
Copy link
Member

tensor-programming commented Apr 9, 2020

This can likely be merged, the changes where made a few weeks ago. Will have to add some windows ps1/batch related commands but yes, its all good to go. (edit) Looks like the PR was actually removed later. You guys can still merge this, the windows support will just have to come after they sort out a mac related issue and remerge the windows PR.

@jbolda
Copy link
Member

jbolda commented Apr 12, 2020

Thanks @DrSensor for taking care of this!

@jbolda jbolda merged commit c649ef4 into dev Apr 12, 2020
@jbolda jbolda deleted the feature/task-runner branch April 12, 2020 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants