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

Fixed: #3928 [Module Examples] Add First Class Python Support #4058

Merged
merged 21 commits into from
Dec 9, 2024

Conversation

himanshumahajan138
Copy link
Contributor

@himanshumahajan138 himanshumahajan138 commented Dec 1, 2024

Pull Request

Added First Class Python Support [Module Examples]
Part of: #3928

Description

Module Examples for Python 1-common-config, 2-custom-tasks, 3-override-tasks, 4-compilation-execution-flags, 5-resources, 6-pex-config Examples

Related Issues

Checklist

  • 1-common-config
  • 2-custom-tasks
  • 3-override-tasks
  • 4-compilation-execution-flags
  • 5-resources
  • 6-pex-config
  • Updated Documentation

Status

Added & Require Review!!!

@himanshumahajan138
Copy link
Contributor Author

(@lihaoyi and @jodersky) Sir Let's Review this PR as Backbone support (Documentation will be updated after finalizing the Code)

Also @lihaoyi guide me for what kind of configs we needs to add for pex config example are we targeting specific bundle args or something else ???

@himanshumahajan138
Copy link
Contributor Author

himanshumahajan138 commented Dec 2, 2024

@lihaoyi Sir please guide me,

Why this error is coming where i have used show command in cli test case???

[3297] /home/runner/work/mill/mill/out/example/pythonlib/module/2-custom-tasks/local/testCached.dest/sandbox/run-2> ./mill --disable-ticker show lineCount
[3297] --- Expected output ----------
[3297] 22
[3297] ------------------------------
[3298] /home/runner/work/mill/mill/out/example/pythonlib/module/1-common-config/local/testCached.dest/sandbox/run-2> ./mill --disable-ticker show bundle
[3298] --- Expected output ----------
[3298] ".../out/bundle.dest/bundle.pex"
[3298] ------------------------------

This happens for both of the test cases
 
[3297]   utest.AssertionError: else assert(evalResult.isSuccess)
[3297]   evalResult: mill.testkit.IntegrationTester.EvalResult = EvalResult(false,mill-server/ exitCode file not found
[3297]   ,)...............more below

These Test Cases are running perfect in local but i am not getting why this error is happening in CI please help...

I got Biggest Annoying thing....

when i added the code under foo folder in the latest commit then it worked for show command what is happening here sir please explain me....

Copy link
Member

@jodersky jodersky left a comment

Choose a reason for hiding this comment

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

In general this looks like it's on the right track.

If you can add explanations to the examples I can give it another look

pythonlib/src/mill/pythonlib/PythonModule.scala Outdated Show resolved Hide resolved
pythonlib/src/mill/pythonlib/PythonModule.scala Outdated Show resolved Hide resolved
example/pythonlib/module/5-resources/foo/test/src/test.py Outdated Show resolved Hide resolved
@himanshumahajan138 himanshumahajan138 changed the title [WIP] Fixes: #3928 [Module Examples] Add First Class Python Support Fixed: #3928 [Module Examples] Add First Class Python Support Dec 5, 2024
@himanshumahajan138
Copy link
Contributor Author

@lihaoyi @jodersky
Ready for Review!

@himanshumahajan138
Copy link
Contributor Author

@lihaoyi @jodersky anyone Up There ???

@lihaoyi
Copy link
Member

lihaoyi commented Dec 8, 2024

@jodersky I'll leave this review to you, feel free to merge this when you think it's in good state and I'll pay out the bounty

Copy link
Member

@jodersky jodersky 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 starting to look good for a merge. I left a couple of comments, but once those are cleared we can merge it.

example/pythonlib/module/1-common-config/build.mill Outdated Show resolved Hide resolved
example/pythonlib/module/6-pex-config/build.mill Outdated Show resolved Hide resolved
example/scalalib/module/2-custom-tasks/build.mill Outdated Show resolved Hide resolved
@himanshumahajan138
Copy link
Contributor Author

@jodersky Let's Finalize this if satisfied

Copy link
Member

@jodersky jodersky left a comment

Choose a reason for hiding this comment

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

Let's merge this

@jodersky jodersky merged commit 99828ea into com-lihaoyi:main Dec 9, 2024
27 checks passed
@jodersky
Copy link
Member

jodersky commented Dec 9, 2024

Thanks!

@himanshumahajan138
Copy link
Contributor Author

himanshumahajan138 commented Dec 9, 2024

Great, @jodersky and @lihaoyi Thanks for your Guidance and support...

@jodersky
Copy link
Member

jodersky commented Dec 9, 2024

@himanshumahajan138, one minor comment: for the following PRs, could you please not say "fixed #3928" in the title, but instead "part of #3928". If you include the words "fixed" or "closes", then github will automatically close the linked issue, which we don't want in this case

@himanshumahajan138
Copy link
Contributor Author

@himanshumahajan138, one minor comment: for the following PRs, could you please not say "fixed #3928" in the title, but instead "part of #3928". If you include the words "fixed" or "closes", then github will automatically close the linked issue, which we don't want in this case

OK Sir 😊 i will take care for future PR's...

@lefou lefou added this to the 0.12.4 milestone Dec 9, 2024
@himanshumahajan138 himanshumahajan138 deleted the issue-3928 branch December 9, 2024 17:32
@lihaoyi
Copy link
Member

lihaoyi commented Dec 9, 2024

Thanks @himanshumahajan138, I'll transfer this portion of the bounty using your existing bank details

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.

4 participants