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

external/build_all.sh script is broken #5657

Open
judovana opened this issue Sep 28, 2024 · 11 comments
Open

external/build_all.sh script is broken #5657

judovana opened this issue Sep 28, 2024 · 11 comments

Comments

@judovana
Copy link
Contributor

Hi!

I do not understand purpose of https://github.com/adoptium/aqa-tests/blob/master/external/build_all.sh. I addition, it seems it is unused, because - per #5553 (comment) - it was broken since 6.February.2024, and nobody noticed.

of course I may be absolutely missing some point, and despite several quiestions on several places, I never got an reply. Looking forward for explanation!

@judovana
Copy link
Contributor Author

Hello, any progress here?

@judovana
Copy link
Contributor Author

Any update please?

@smlambert
Copy link
Contributor

You seem to have an inconsistent perspective on removing versus keeping files that are not used. You want to remove this one (which was at one point in the past used by someone for local testing), but you argue to keep this other one #5553 (comment). I will also note that your tendency to disagree/argue and push back on review requests causes reviewers to not want to review your PRs since you will not make the changes they ask for.

I prefer that we first look at fixing the functionality of build_all.sh, rather than removing the script, and also prefer that every PR is related to an issue that describes the problem along with any discussion regarding potential options for resolving it.

@judovana
Copy link
Contributor Author

judovana commented Nov 19, 2024

Have you read the linked comment in initial description? The file is broken for an year.

I had clearly written that I will remove it if you insists, but I had kindly asked to reconsider, because the file is quite useful. I'm always doing what reviewer asks for, but not blindly. If I see an reason in that, I try to highlight that. If that is not persuasive enough, I follow what reviwer wanted. You may see that pattern in all my PRs.

I know you want issue for evrey PR, so I'm doing my best to make it. This is an issue.

@judovana
Copy link
Contributor Author

Also You had promised (sorry, no written evidence) that you will ask other users. "any update" means, if you were able to keep your word, and did what you promised.

I think it is quite common that contributor and reviewer do not agree, and then discussion follows. Of course reviewer have final word, and can always say that it is not discussable . In live projects, where is more active reviewers, it is also quite common that there is more opinions, unluckily, this diversity is missing here.

@smlambert
Copy link
Contributor

yes, I looked back at how the file was used previously, thus my answer above "in the past used by someone for local testing" and see the value of being able to generate all files at once (especially if we consider changing how we run these tests in the future by building and caching images and pulling them for testing).

@judovana
Copy link
Contributor Author

So you really wish to keep broken file, for the unlikely case, that in some future, it may be used again? The file is quite trivial, and in case it will be really for some usage, then it will be quite easy to write it again, and reflect the state of the moment when it is reincarnated.

@judovana
Copy link
Contributor Author

"I prefer that we first look at fixing the functionality of build_all.sh, rather than removing the script, and also prefer that every PR is related to an issue that describes the problem along with any discussion regarding potential options for resolving it."

It is valid point. I would really like you to ask to all possible users, if it is really used. I doubt, because it is broken so long. The fix is simple - to iterate over versions, or as I put it to #5553 to enforce the usage of version. The iteration over versions seems no longer possible, as the support matrix is no longer symmetric. Thus build_all remains bound to #5553, which is step allowing to iterate against practically anything. Worst of it all is keeping unused, dead code.

@judovana
Copy link
Contributor Author

Thanx for the #5763! That is very nice info which gives me a bit more info to build on top of. From that point of view I'm definitely ok to close this one. Please do if you wish. I will close on few days time-out if nothing changes.

@smlambert smlambert changed the title remove external/build_all.sh script external/build_all.sh script is broken Nov 20, 2024
@smlambert
Copy link
Contributor

Changing the title of this issue, so the outcome can be fix the script instead of remove, as needed (based on 5763 enquiry)

@judovana
Copy link
Contributor Author

Note that it will be "fixed" by #5553 's 26cd87f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

No branches or pull requests

2 participants