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

Suggest: Hide required elements and comments #1103

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

baltpeter
Copy link
Member

We never know whether people are just guessing for the required elements and usually end up removing them anyway for that reason.

And as for the comments: Records should only have comments in very rare cases. This field just encouraged people to leave the kinds of comments that should be left on the company pages instead.

We never know whether people are just guessing for the
required elements and usually end up removing them anyway
for that reason.

And as for the comments: Records should only have comments
in very rare cases. This field just encouraged people to
leave the kinds of comments that should be left on the
company pages instead.
@cypress
Copy link

cypress bot commented Sep 19, 2023

Passing run #4985 ↗︎

0 80 8 0 Flakiness 0

Details:

Suggest: Hide required elements and comments
Project: datenanfragen/website Commit: 54efeb5200
Status: Passed Duration: 03:50 💡
Started: Sep 19, 2023 2:33 PM Ended: Sep 19, 2023 2:37 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Member

@mal-tee mal-tee left a comment

Choose a reason for hiding this comment

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

And as for the comments: Records should only have comments in very rare cases. This field just encouraged people to leave the kinds of comments that should be left on the company pages instead.

I don't think I agree with this. Sometimes people leave comments with further information, which could fit into our scheme, but we can not expect that everybody adheres to our rules when they want to add a company (for the first time). I think its important to have some "freestyle" comment field for suggest. It doesn't have to be the schema.json comment ofc.

@baltpeter
Copy link
Member Author

baltpeter commented Sep 19, 2023

Exactly, I'm fine with having a free-form comment field that just posts comments in the GitHub PR. But having those go to the record comments is just wrong (and people regularly misread the current comment field as such).

@zner0L
Copy link
Member

zner0L commented Sep 28, 2023

Would you mind opening an issue for that then, so that we can implement this later?

@mal-tee
Copy link
Member

mal-tee commented Sep 28, 2023

If we postpone the new comment feature, we will never implement it. A PR that disables the status quo comments should introduce a new comment feature as well imo.

@christian-weiss
Copy link
Contributor

Do i get it right, the following change is requested:

  • Update "PR creator" to include the comments in PR text
  • drop "comments" from "file creator" (for PR) (there is no need to persist the comments in company records)
  • drop "comments" from schema.json (there is no need to persist the comments in company records)
  • update "comments" field from all records in "data" repo to make them fit to the new schema
  • drop code that read/write/access the "comments" field

right?

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

Successfully merging this pull request may close these issues.

4 participants