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 note that custom decorators are not supported yet #2539

Closed
wants to merge 2 commits into from

Conversation

KarateCowboy
Copy link
Contributor

No description provided.

@KarateCowboy
Copy link
Contributor Author

KarateCowboy commented May 5, 2024

Hi. It's a miniscule change, but would have saved me a good hour, had it been there. Had to query in the Discord about why this code was compiling and running, but all that printed was 'Hallo'

 fn logger_thing[func: fn () -> None]() -> fn () -> None:
  fn inner():
    print("you are decorative")
    func()
  return inner 

@logger_thing
fn say_hallo():
  print("Hallo")


fn main():
  say_hallo()

Oh, and, I'm new to this DCO and 'signed-off' stuff.

@KarateCowboy KarateCowboy changed the base branch from main to nightly May 6, 2024 00:01
@KarateCowboy KarateCowboy requested review from jackos and a team as code owners May 6, 2024 00:01
@KarateCowboy KarateCowboy changed the base branch from nightly to main May 6, 2024 00:02
@gabrieldemarmiesse
Copy link
Contributor

Thanks for the pull request, indeed, that will help other users. If you want I can help you out.

  • Concerning the DCO, the instructions are here to fix an existing PR: https://github.com/modularml/mojo/pull/2539/checks?check_run_id=24614427145 . I don't recommend doing this now since you'll need to open another PR. See below:
  • Another thing that needs to be fixed: the target branch should be nightly, no main. Here you target main. Feel free to close this PR and open a new PR targeting the nightly branch. When you commit on this new branch, use git commit -s and that will ensure that you're not bothered by the DCO.

@arthurevans
Copy link
Collaborator

Thanks for the PR! Seconding what @gabrieldemarmiesse said, if you can sign the commits and create a new PR against main, we can help you get this landed.

@KarateCowboy
Copy link
Contributor Author

Great great. You guys are fast. i'll put together a new PR this evening after work

@ematejska ematejska added the mojo-repo Tag all issues with this label label May 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mojo-repo Tag all issues with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants