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 "feat: bugfix for artifacts upload (#749)" #766

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

pranav-firake
Copy link
Contributor

@pranav-firake pranav-firake commented Jan 31, 2025

Problem

  • CodeFIles list is missing from Requirements.json.
  • Requirements.json gets created as artifact when user starts transformation

Some changes with PR #749 introduced delays in writing codeFiles list to requirements.json

We do not need codeFiles from requirements.json for transformation job but it is needed for summary generation.
On IDE, this summary is parsed to update counts, diffs for file changes

Solution

This reverts commit 71c0a19.

Test

Verified that after reverting this commit, we could get successfully .bak files in view diff screen
image

failing test CI job does not seem to be connected with nettransform. will connect with flare team for more guidance on it.
when I run npm run test locally, gave me all successful executions

failing test is already failing with mainline commit - https://github.com/aws/language-servers/actions/runs/13075234433/job/36485706871

1) crossFileContextUtil
       getCrossFileCandidate
         should return opened files, exclude test files and sorted ascendingly by file distance:
     TypeError: Invalid URL
      at new NodeError (node:internal/errors:405:5)
      at new URL (node:internal/url:676:13)
      at D:\a\language-servers\language-servers\server\aws-lsp-codewhisperer\src\language-server\utilities\supplementalContextUtil\crossFileContextUtil.ts:241:29
      at Array.filter (<anonymous>)
      at Object.getCrossFileCandidates (src\language-server\utilities\supplementalContextUtil\crossFileContextUtil.ts:236:10)
      at Context.<anonymous> (src\language-server\utilities\supplementalContextUtil\crossFileContextUtil.test.ts:137:28)
image

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@pranav-firake pranav-firake requested a review from a team as a code owner January 31, 2025 22:03
Copy link

@kanikaul-amazon kanikaul-amazon left a comment

Choose a reason for hiding this comment

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

Let's create an issue to follow up with the stream fix, thanks

@xueningl-az xueningl-az self-requested a review February 1, 2025 00:32
@ege0zcan
Copy link
Contributor

ege0zcan commented Feb 3, 2025

Failing test can be ignored, it is a separate issue which we are investigating. Feel free to merge

@dogusata dogusata enabled auto-merge (squash) February 3, 2025 13:02
@ege0zcan ege0zcan removed the request for review from xueningl-az February 3, 2025 13:59
@ege0zcan ege0zcan disabled auto-merge February 3, 2025 13:59
@ege0zcan ege0zcan merged commit 0c07a17 into main Feb 3, 2025
5 of 6 checks passed
@ege0zcan ege0zcan deleted the pranavfi-reverting-file-stream-change branch February 3, 2025 14:00
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