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

[distribution] do not ship *.class files in test/ for source dist #1104

Merged
merged 1 commit into from
Nov 1, 2022

Conversation

marscher
Copy link
Member

fixes #1101

@codecov
Copy link

codecov bot commented Oct 31, 2022

Codecov Report

Merging #1104 (e16affe) into master (c139725) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1104      +/-   ##
==========================================
- Coverage   88.67%   88.65%   -0.02%     
==========================================
  Files         111      111              
  Lines       10207    10207              
  Branches     4014     4014              
==========================================
- Hits         9051     9049       -2     
- Misses        698      699       +1     
- Partials      458      459       +1     
Impacted Files Coverage Δ
native/common/jp_method.cpp 97.68% <0.00%> (-1.16%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Thrameos
Copy link
Contributor

@marscher Are you sure those .class files were not used during the distribution build process? I remember at one point the build system was doing something in which javac was not installed (just the headers and the jre) and we had to run the tests. If that is the case then I guess we can just have a separate tar ball for the .class files. But you may want to trigger the distribution pipeline to make sure it doesn't go bust before commiting this change.

@marscher
Copy link
Member Author

marscher commented Nov 1, 2022

Thanks for noticing that, I wasn't aware. Is it safe to run this pipeline without actually releasing something we don't want to?

@Thrameos
Copy link
Contributor

Thrameos commented Nov 1, 2022

Sure I trigger the release one on azure anytime I want to see if the release works. It just makes a bunch of artifacts. Since the pipeline isnt tied to PyPI with a push (I copy and check the wheels manually) you are free to test as much as you need.

@marscher
Copy link
Member Author

marscher commented Nov 1, 2022

I tried pushing this branch to origin/releases/test, but it seems the pipeline didn't run. Unfortunately I cannot login via Github to Azure, just receiving an error message from their side.

@Thrameos
Copy link
Contributor

Thrameos commented Nov 1, 2022

Darn. I was worried about that. I know that I granted credentials to run the pipeline, but there was always a problem with your account. If you can send me the azure login name I can try to refresh it.

As for now I did a manual trigger. Should complete in about 10 to 15 minutes.

https://dev.azure.com/jpype-project/jpype/_build/results?buildId=1056&view=results

@Thrameos
Copy link
Contributor

Thrameos commented Nov 1, 2022

Looks like this one is safe to merge. Our pipelines have improved since the our move to azure and it looks like we don't need the class files to test the packages anymore.

LGTM!

@Thrameos Thrameos self-requested a review November 1, 2022 15:52
@Thrameos Thrameos closed this Nov 1, 2022
@Thrameos Thrameos reopened this Nov 1, 2022
@Thrameos Thrameos merged commit 4bacf4c into jpype-project:master Nov 1, 2022
@marscher marscher deleted the update_manifest branch November 3, 2022 09:46
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.

source distribution includes test binaries (e.g. class/jar files)
2 participants