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

Revert "Add latest and trending builds list to the build list module … #7951

Conversation

kwjanssen
Copy link

@kwjanssen kwjanssen commented Jul 25, 2024

Reverts (#7389)

This reverts commit 8bea7bd.

Fixes #7944 #7946 #7943 #7941 .

Description of the problem being solved:

Removes integration with 3rd parties that could be abused. See various discussions in issues for more context.

Steps taken to verify a working solution:

  • Revert

Link to a build that showcases this PR:

N/A

Before screenshot:

before revert

After screenshot:

after revert

@enpinzolas
Copy link

I would not be necessarily against the similar builds feature as long as it can aggregate from more APIs (poe-vault, maxroll, etc...) since the sorting algorithm is based from similarity (passives, items, skills) instead of some potentially PTW feature, but the trending page really should just go.

@snt0
Copy link

snt0 commented Jul 25, 2024

Better integration would involve the user initiating the search. Maybe a "find similar builds" box that matches on common skills?

@niklasstich
Copy link

niklasstich commented Jul 25, 2024

The fact that this is enabled by default and you seemingly can't deactivate it at all is ridiculous. Why is there no "None" option in the drop down (which should be selected by default)?
image
This is some crazy entitlement for an open source project.

Edit:
I correct myself, you can turn it off in the settings
image
Still not happy about it.

@mcarabolante
Copy link

Let's say that by default there shouldn't be any site selected, it should be up to the user to decide which one he wants to use as a datasource, that would deal with one issue.

However, there is another major one, who, based on which discretions will define which site should or not be integrated with PoB, reason I ask this, there are certains sites that are known for offering low quality/non functional builds, should they be allowed in PoB? If not why? What is worse there are known ones that does RMT.

While I dont mind the UI/UX, the feature itself looks great and can be very helpful for newcomers, the datasource issue might generate enough issue to make it not viable

@Orden4
Copy link

Orden4 commented Jul 25, 2024

However, there is another major one, who, based on which discretions will define which site should or not be integrated with PoB

The completely hands-off approach would just be to not provide any options and just require users to add sites via plugins/API links hosted on their site. Like that, it is entirely up to the users discretion which sites they are interested in seeing builds from. That being said, I have no clue if this can be done in an easy/safe manner.

@sirgregoryadams
Copy link

All the arguments here are valid, however, this should all be discussed before the feature is added. So, until all of this has been clarified, I strongly believe that it should be removed.

@Wires77
Copy link
Member

Wires77 commented Jul 25, 2024

All the arguments here are valid, however, this should all be discussed before the feature is added. So, until all of this has been clarified, I strongly believe that it should be removed.

To be fair, the PR that this is reverting was open for 5 months without any responses other than mine.

@sirgregoryadams
Copy link

All the arguments here are valid, however, this should all be discussed before the feature is added. So, until all of this has been clarified, I strongly believe that it should be removed.

To be fair, the PR that this is reverting was open for 5 months without any responses other than mine.

For sure. Absolutely no blame at all. What I'm suggesting is to remove the feature for now, and have that discussion in the context of the reaction the feature received. That's all.

@MagerlinC
Copy link

I agree that this feature has an odd feel to it. While I understand the idea of new-player appeal for browsing builds, there are other tools out there for that, and most people I have talked to so far were primarily mildly shocked to see clickbait titles all over their build planning tool. Maybe this could be improved with other resources for builds added, but for now, I would also be in favor of reverting.

If there are also financial ties to the platform in question.... Well.

Copy link

@murphyfa murphyfa left a comment

Choose a reason for hiding this comment

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

This feature was added with the intent of personal gain by the author and should be reverted immediately. For the integrity of the project, none of this author's code should be trusted or kept. If there's any room for discussing a similar, community-driven feature, that should be proposed separately.

LGTM 👍

@0x42424242
Copy link

Just want to add support for removing this feature. Path of Building is amazing software, and it does it's job - build calculation - fantastically. Adding the amalgamation of public builds and a browsing feature feels like a significant step away from it's original purpose.

Sometimes, in my opinion, it's much better for a tool to do one job, and do it exceedingly well, rather than scope creep out into different areas. Often these secondary features are only half baked, and add significant maintenance overhead to the project. It really feels like separate websites, whoever they are (maxroll, pob archives, poe vault, poe ninja) are a more suitable place to take care of this amalgamation problem.

Please don't go down the "Path of Building has to be the only needed tool" path.

With that said, I acknowledge that nothing was said on the original PR so I don't feel like this is the merger's fault in any way.

@aymendps
Copy link

At first, I liked the idea, but after considering the unnecessary risks/maintenance the project will have to take, and the lack of transparency behind the PoB Archives website owner, I believe this feature has to be reverted. If there is a adjusted version to be added in the future, I suggest one that does not have the "trending" space, or any space that encourages monetized content/ads. But rather a "search" space where the user can search for builds by skill/name/creator. The search can then be conducted through multiple providers that are trusted by the community, and will only be there as an opt-in feature for the user to LOOK for something specific, rather than getting bombarded by rmt biased boosted builds.

@PPFilip
Copy link

PPFilip commented Jul 26, 2024

All the arguments here are valid, however, this should all be discussed before the feature is added. So, until all of this has been clarified, I strongly believe that it should be removed.

To be fair, the PR that this is reverting was open for 5 months without any responses other than mine.

Which also means, that the guy pushed it few daysweeks after he started scraping all reddits and pester people with his bot. I'd say the lack of interest then vs now, is because regular users (me included, I am not a contributor here) usually won't follow repos for changes to comment on - for better or worse.

If this feature is to stay, it should be done in less intrusive way - disabled by default (at the risk of getting lost though) or enabled but with no site preselected (with a text saying you can select a provider in dropdown or smth), so people are not giving telemetry to a random 3rd party without consent the moment they start pob. I'd argue that I do not mind hitting pob update link on start, or load data when I press import build, but the act of calling extra API on launch that is not necessary to function or run the game is a bit too intrusive.

@Moremackles
Copy link
Contributor

Adding my two cents: It's a cool feature, but other replies here have convinced me it is not handled the best way. I would rather it be removed then implemented the way it is currently since it's unclear who would take up the mantle of refactoring it to be less intrusive. That being said, I can see the upsides and I wouldn't be upset if it stayed.

@calmbook1300
Copy link

Please respect your users and don't fill PoB with clickbait.

@jjstewart
Copy link

Is this going to get merged?

@Nightblade
Copy link
Contributor

Latest / trending builds list is currently disabled, see #7995

@kwjanssen kwjanssen closed this Aug 2, 2024
@kwjanssen kwjanssen deleted the kwjanssen/revert-trending-builds branch August 2, 2024 23:08
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.

Trending Builds section should be not be present.