Skip to content

Find usages add#68

Open
xzescha wants to merge 45 commits into
objectionary:masterfrom
xzescha:find-usages-add
Open

Find usages add#68
xzescha wants to merge 45 commits into
objectionary:masterfrom
xzescha:find-usages-add

Conversation

@xzescha

@xzescha xzescha commented May 13, 2025

Copy link
Copy Markdown
Contributor

Added find usages add-on implementation. It's not fully refactored yet, but till tomorrow it'll be fully ready to merge!

@0crat

0crat commented May 14, 2025

Copy link
Copy Markdown

@xzescha Hey there! 👋 I noticed your branch name "find-usages-add" doesn't quite follow our naming convention. According to our policy, this means a -12 point penalty. For future reference, it's best to name branches after the ticket number you're working on, like "67" in this case. This helps everyone stay organized and on the same page. Your current balance is now -12. Let's aim for better naming next time to avoid those penalties!

@xzescha

xzescha commented May 17, 2025

Copy link
Copy Markdown
Contributor Author

@yegor256 could you please review this and my other PR. Hope so, they're valid and ready to merge!

@yegor256

Copy link
Copy Markdown
Member

@xzescha I see most build jobs failing. We can't merge now, see: https://www.yegor256.com/2025/04/19/dont-merge-into-broken-master.html

@codecov

codecov Bot commented May 30, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 48.91304% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 35.10%. Comparing base (5a07317) to head (c18c372).
⚠️ Report is 20 commits behind head on master.

Files with missing lines Patch % Lines
...ain/java/org/eolang/jetbrains/EoCompileAction.java 27.50% 28 Missing and 1 partial ⚠️
...ava/org/eolang/jetbrains/EoFindUsagesProvider.java 0.00% 17 Missing ⚠️
.../org/eolang/jetbrains/EoCompletionContributor.java 97.14% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master      #68      +/-   ##
============================================
- Coverage     35.60%   35.10%   -0.50%     
+ Complexity       25       19       -6     
============================================
  Files            13       12       -1     
  Lines           191      188       -3     
  Branches          5       17      +12     
============================================
- Hits             68       66       -2     
  Misses          119      119              
+ Partials          4        3       -1     
Flag Coverage Δ
junit 35.10% <48.91%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@xzescha

xzescha commented Jun 1, 2025

Copy link
Copy Markdown
Contributor Author

@yegor256, Done! I've fixed pipelines, added few tests for new funtionality.

@yegor256

yegor256 commented Jun 1, 2025

Copy link
Copy Markdown
Member

@volodya-lombrozo can you please review this?

@yegor256 yegor256 requested a review from volodya-lombrozo June 1, 2025 03:51

@volodya-lombrozo volodya-lombrozo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@xzescha Great thanks for the PR. There are lots of valuable changes.

However, I highly recommend you reading Helping others review your changes. Pay attention to:

  1. Write small pull requests
  2. Provide context and guidance

For example, it's hard to understand what you are trying to solve in this PR. Could you provide a link to the issue, please? Also, a detailed explanation would also greatly help here.

Another important aspect is the PR size. The PR is too big and it seems we are trying to solve many problems at once. Can we split this PR on a few small PRs? It would be better if they were 100~300 lines length.

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.

4 participants