+
Skip to content

standard CI envs dir & naming conventions #95

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

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jGaboardi
Copy link
Member

@jGaboardi jGaboardi commented Jul 12, 2025

Copy link

codecov bot commented Jul 12, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.2%. Comparing base (015e8bb) to head (fb51eef).
Report is 22 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main     #95     +/-   ##
=======================================
+ Coverage   95.0%   96.2%   +1.2%     
=======================================
  Files          6       7      +1     
  Lines        480     473      -7     
  Branches     109       0    -109     
=======================================
- Hits         456     455      -1     
- Misses        14      18      +4     
+ Partials      10       0     -10     

see 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jGaboardi jGaboardi self-assigned this Jul 12, 2025
@jGaboardi jGaboardi changed the title ci dir should not be hidden standard CI envs dir & naming conventions Jul 12, 2025
Comment on lines +31 to +35
py311_access-oldest,
py311_access-latest,
py312_access-latest,
py313_access-latest,
py313_access-dev,
Copy link
Member

Choose a reason for hiding this comment

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

I suppose this was discussed during the dev call? Imho it is redundant given it lives in access repo but have no strong feelings.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, see here. The decision was basically "let James do his thing unless it is absolutely insane."

And Germano and I discussed here in spopt

Copy link
Member

Choose a reason for hiding this comment

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

My main issue is that this is how it looks on GitHub. I have to open each to know which env has failed.

Screenshot 2025-07-17 at 15 52 49

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is also by design so it is easier to keep track of exactly which has failed.

Copy link
Member

Choose a reason for hiding this comment

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

What? I don't see the suffixes here, how is that by design?

Copy link
Member Author

@jGaboardi jGaboardi Jul 17, 2025

Choose a reason for hiding this comment

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

I think we are talking about 2 different things. I talking simply about the file name of the testing environments.

Do you mean that the label is longer so it is truncated? Maybe that's simply a browser issue because this is how I see:

Wrong screenshot

Copy link
Member Author

Choose a reason for hiding this comment

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

I concede your point @martinfleis

Screenshot 2025-07-17 at 10 04 31 AM

Copy link
Member

Choose a reason for hiding this comment

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

You are looking at the result through the Actions tab but this is how it looks like from within the PR. Irrespective of the cause, this change is now making the interaction more complicated while providing little to no value. I am not maintaining access or spopt but if this is supposed to be new default across the federation, I'd like to have a further discussion about it.

Screenshot 2025-07-17 at 16 05 36

Copy link
Member

Choose a reason for hiding this comment

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

It also make copy&paste between subprojects more work as you need to replace the names.

Copy link
Member Author

Choose a reason for hiding this comment

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

You are looking at the result through the Actions tab but this is how it looks like from within the PR. Irrespective of the cause, this change is now making the interaction more complicated while providing little to no value.

I already conceded your point there. I had not realized previously that the truncation was happening everywhere on GH. That was my mistake.

It also make copy&paste between subprojects more work as you need to replace the names.

This was my intention – making things explicit to avoid copy&paste mistakes.

I'd like to have a further discussion about it.

I am absolutely on board with more discussion and ideas better than mine. Let's not merge this PR. I'll look back into ones I previously did for spopt etc. as time permits. And also pause on implementing these updates in other packages for now.

@jGaboardi
Copy link
Member Author

@martinfleis can you give an explicit approval here (if you do so approve)?

@jGaboardi jGaboardi marked this pull request as draft July 17, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
点击 这是indexloc提供的php浏览器服务,不要输入任何密码和下载