Skip to content

bug(#209): better defect message in mandatory-home#214

Merged
yegor256 merged 6 commits intoobjectionary:masterfrom
h1alexbel:209
Jan 10, 2025
Merged

bug(#209): better defect message in mandatory-home#214
yegor256 merged 6 commits intoobjectionary:masterfrom
h1alexbel:209

Conversation

@h1alexbel
Copy link
Member

In this pull I've updated defect message reported by mandatory-home lint, now it says about the need of home URL.

see: #209

@h1alexbel
Copy link
Member Author

waiting for #213

@h1alexbel
Copy link
Member Author

@volodya-lombrozo could you please check this one?

Copy link
Member

@volodya-lombrozo volodya-lombrozo left a comment

Choose a reason for hiding this comment

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

@h1alexbel Thank you for the contribution! It's a bit more clear now, but for me it's not sufficient because I don't know what to do with the url and home meta.

Should it be <home url="x"/> or <meta><home><url>x</url></home></meta> or <meta home="x"/>? It's impossible to say from this message.

@yegor256 Can you help us with this, please? Can we add a link to the description or at least something.

@yegor256
Copy link
Member

@volodya-lombrozo we can't explain in one error message how XMIR format is designed. It's expected that the user has certain background knowledge about XMIR and knows what is "meta". We can add information about XMIR into the Javadoc of the Defect class, it may help (but it's a separate problem/ticket).

@volodya-lombrozo
Copy link
Member

@volodya-lombrozo we can't explain in one error message how XMIR format is designed. It's expected that the user has certain background knowledge about XMIR and knows what is "meta". We can add information about XMIR into the Javadoc of the Defect class, it may help (but it's a separate problem/ticket).

@yegor256 Maybe we can add a single link to the documentation of XMIR where I will be able to find information about meta objects? Right now I have no idea where to find it. I see only cryptic messages that tell almost nothing.

It will look like:

1. Cryptic error [name-outside-of-abstract-object]
2. Cryptic error [mandatory-home]

XMIR docs is here: URL 

Otherwise, this PR doesn't solve the original issue.

@h1alexbel
Copy link
Member Author

@volodya-lombrozo @yegor256 I think publishing (but not linking actually) our lint motives will help with the understanding what needs to be fixed:

1. Cryptic error [name-outside-of-abstract-object]
2. Cryptic error [mandatory-home]

Lints list is here: URL
XMIR docs is here: URL

So the user will find the rule id, let's say: mandatory-home, go to the lints website, find it and read about it.

Currently we have them here: https://github.com/objectionary/lints/tree/gh-pages/_site/0.0.31, but their are not accessible through website. WDYT?

@yegor256
Copy link
Member

@volodya-lombrozo I believe, the links are added in this PR: #229

@yegor256 yegor256 merged commit 9815f0d into objectionary:master Jan 10, 2025
12 checks passed
@yegor256
Copy link
Member

@h1alexbel this contribution is good, thanks!

@yegor256
Copy link
Member

@volodya-lombrozo as a general rule, a merge PR doesn't mean a closed issue. When PR is good enough to be merged, we should accept it and merge ASAP. Then, we get back to the ticket and discuss whether it may be closed. If not, we make another PR, and so on. "This PR doesn't solve the problem" is not a valid reason for its rejection.

@h1alexbel h1alexbel deleted the 209 branch January 10, 2025 12:06
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