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

Script to clean showcase.json #3851

Merged
merged 3 commits into from
Dec 4, 2024
Merged

Script to clean showcase.json #3851

merged 3 commits into from
Dec 4, 2024

Conversation

lmuntaner
Copy link
Contributor

Motivation

The showcase.json is very long and cleaning up abandoned dapps is painful and a long process.

In this PR, I introduce a script to be run which checks a few things to clean up the showcase:

  • Do they really use II? A project with the website pointing to the repo can't use II.
  • Is the website working?

The changes made by this script should be reviewed by a person.

Changes

  • New script to clean up showcase.json
  • Changes in showcase.json after running the script.

@lmuntaner lmuntaner requested a review from a team as a code owner December 4, 2024 11:39
@lmuntaner lmuntaner marked this pull request as draft December 4, 2024 11:39
@meodai
Copy link
Contributor

meodai commented Dec 4, 2024

I appreciate the effort to streamline the process of cleaning up the showcase, but I think this approach has some issues. Specifically, checking if a site is working or if it uses II based on a single run seems unreliable. Any of these sites could be unreachable for various reasons—temporary downtime, server issues, or even a bad network on the script's end.

If we were to proceed with this method, I think it would be more robust to sample these checks over time instead of relying on a one-time test. This way, we could avoid mistakenly flagging legitimate projects as abandoned.

Do you know who owns the showcase project? We should talk to whoever this is.

showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
showcase.json Outdated Show resolved Hide resolved
@lmuntaner
Copy link
Contributor Author

I appreciate the effort to streamline the process of cleaning up the showcase, but I think this approach has some issues. Specifically, checking if a site is working or if it uses II based on a single run seems unreliable. Any of these sites could be unreachable for various reasons—temporary downtime, server issues, or even a bad network on the script's end.

If we were to proceed with this method, I think it would be more robust to sample these checks over time instead of relying on a one-time test. This way, we could avoid mistakenly flagging legitimate projects as abandoned.

Do you know who owns the showcase project? We should talk to whoever this is.

I agree with you, this script is far from perfect.

I hope this is just a first step to help whoever cleans up the showcase @jennifertrin by flagging projects susceptible to be removed. The output of this script should be by no means considered as final. It's a first step to help cleaning up.

Ideally, this script will be expanded with the learnings of the manual process and reviewing of the cleaned up projects.

That's why I didn't put it in a Github action or anything like that. It's purely a helper for whoever cleans up the showcase.

@lmuntaner
Copy link
Contributor Author

Hi @jennifertrin I went one per one of the dapps suggested to be removed:

"https://seers.social/" 405 - don't remove
"https://yrog5-xqaaa-aaaap-qa5za-cai.ic0.app/#/swap" 451 - don't remove
"https://hl2zz-gyaaa-aaaad-qas3a-cai.raw.ic0.app/" 502 - ok to remove
"https://t5mcf-cqaaa-aaaag-qcjna-cai.raw.icp0.io/" 404 - don't remove
"https://sama.network/" 405 - ok to remove
"https://www.taxlint.online/" 405 - ok to remove
"https://onzmk-taaaa-aaaal-acw4a-cai.icp0.io/" 500 - don't remove
"https://allkinds.xyz/" 405 - ok to remove
"https://myordinals.loan/" 500 - ok to remove
"https://fkkq7-siaaa-aaaap-qaaya-cai.ic0.app/" 502 - don't remove
"https://dappradar.com/" 403 - don't remove
"https://alpha.bridge23.app/login" 500 - ok to remove
"https://alpha-gate23.bridge23.app/" 500 - don't remove
"https://uqjdj-siaaa-aaaag-aaoxq-cai.icp0.io" 502 - ok to remove
"https://ic-toolkit.app" 500 - don't remove

The script checks the status when making a curl request. This is far from perfect, but it's better than nothing.

I can't find a rule that is 100% identifying the abandoned dapps. Some have 4xx status, others 5xx status. Same with the dapps that are not abandoned. For different reasons, they give those statuses.

I think that the output of the script should be taken as a starting point of the investigation. Maybe with time it can be fine-tuned by adding a whitelist... This should be owned by whoever cleans up the showcase.

ChatGPT is a wonderful companion to help with scripts. I used it for this script, I'm not a bash expert at all.

The process is:

  1. Run script.
  2. Script flags some dapps.
  3. Investigate all the dapps flagged by the script.
  4. Revert the changes when the dapp is not abandoned.
  5. Leave the rest.
  6. Make PR to clean up.

I will revert the project that you say are not abandoned for this PR.

jennifertrin
jennifertrin previously approved these changes Dec 4, 2024
@jennifertrin jennifertrin self-requested a review December 4, 2024 16:50
@jennifertrin jennifertrin dismissed their stale review December 4, 2024 16:51

Need more time to think of solution

@jennifertrin
Copy link
Contributor

I appreciate the effort to streamline the process of cleaning up the showcase, but I think this approach has some issues. Specifically, checking if a site is working or if it uses II based on a single run seems unreliable. Any of these sites could be unreachable for various reasons—temporary downtime, server issues, or even a bad network on the script's end.

If we were to proceed with this method, I think it would be more robust to sample these checks over time instead of relying on a one-time test. This way, we could avoid mistakenly flagging legitimate projects as abandoned.

Do you know who owns the showcase project? We should talk to whoever this is.

I also

I appreciate the effort to streamline the process of cleaning up the showcase, but I think this approach has some issues. Specifically, checking if a site is working or if it uses II based on a single run seems unreliable. Any of these sites could be unreachable for various reasons—temporary downtime, server issues, or even a bad network on the script's end.
If we were to proceed with this method, I think it would be more robust to sample these checks over time instead of relying on a one-time test. This way, we could avoid mistakenly flagging legitimate projects as abandoned.
Do you know who owns the showcase project? We should talk to whoever this is.

I agree with you, this script is far from perfect.

I hope this is just a first step to help whoever cleans up the showcase @jennifertrin by flagging projects susceptible to be removed. The output of this script should be by no means considered as final. It's a first step to help cleaning up.

Ideally, this script will be expanded with the learnings of the manual process and reviewing of the cleaned up projects.

That's why I didn't put it in a Github action or anything like that. It's purely a helper for whoever cleans up the showcase.

Hello @lmuntaner, I agree with @meodai that the script is far from perfect; from the first pass, some projects should not have been deleted. However, I do think that we should use it (for now) as you mentioned 1) it has to be manually run 2) the team member running the script still has to open a PR and get it approved.

@lmuntaner lmuntaner marked this pull request as ready for review December 4, 2024 19:07
@lmuntaner lmuntaner force-pushed the lm-script-clean--showcase branch from 4285629 to 9ef4947 Compare December 4, 2024 19:08
@lmuntaner lmuntaner merged commit d0158fe into master Dec 4, 2024
4 checks passed
@lmuntaner lmuntaner deleted the lm-script-clean--showcase branch December 4, 2024 19:30
@meodai
Copy link
Contributor

meodai commented Dec 5, 2024

but then why even have it in the repo? Without this context someone might run it...

@lmuntaner
Copy link
Contributor Author

but then why even have it in the repo? Without this context someone might run it...

The PR still needs to be approved.

@meodai
Copy link
Contributor

meodai commented Dec 5, 2024

@lmuntaner well looks very merged to me d0158fe

@lmuntaner
Copy link
Contributor Author

@lmuntaner well looks very merged to me d0158fe

The PR was approved... what do you mean?

@meodai
Copy link
Contributor

meodai commented Dec 5, 2024

@lmuntaner 1h ago you said:

but then why even have it in the repo? Without this context someone might run it...

The PR still needs to be approved.

@lmuntaner
Copy link
Contributor Author

@lmuntaner 1h ago you said:

but then why even have it in the repo? Without this context someone might run it...

The PR still needs to be approved.

Ah, I meant from your comment that "someone might run it". Running it doesn't affect the code because the PR with the changes would still need to be approved 😄

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.

3 participants