FIX 701 #750
Merged
FIX 701 #750
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rking while compiling.
…n every orther. Remove whitespaces before upload. Alert user if OS value is not a valid OS.
… Only insert comments if it doesn't match with matchine os or host or env. Uncomment line before write if it matched.
… More redeable settings json for testing.
…ted. Get OS from OsType enum. Remove os.hostName()
…commas are removed. Must check this. If not valid OS is detected inform user. Added function to remove comments from text.
…"Launch Test". Remove javascript files.
Get the local content and extract the ignored lines before writing the new settings. Add the ignored lines at the beginning of the settings object.
Instead of replacing entires blocks, we will loop each line in the settings file. For each line we are checking if should be ignored or if comments must be toggled. There is a block of code that is repeated to ensure the readability of the code.
shanalikhan
reviewed
Jan 14, 2019
| @@ -13,33 +13,12 @@ describe("Process before upload", function() { | |||
| ); | |||
| }); | |||
|
|
|||
| it("should remove @sync-ignore and @sync ignore lines", () => { | |||
There was a problem hiding this comment.
may i know why these tests were removed.
also can you add test for multi settings
There was a problem hiding this comment.
It was removed since the new approach doesn't need separated methods to accomplish the same and some methods were removed.
I added a new test for multi-line settings but I can't run the test because TSLint throws errors in "commons.ts" file. Check the latest commits.
|
Cool! I'm finishing my latest tasks at my current job an then I will be
free for one or two weeks.
This weekend I will look some other issues to help with since I got
familiar with all the code sync code base.
Kind regards
…On Tue, Jan 15, 2019 at 6:38 AM Shan Khan ***@***.***> wrote:
Merged #750 <#750>
into v3.2.5.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#750 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/Ap0umaGw_P0Ttg4zoXpp5v4q77xYN0Ozks5vDaGzgaJpZM4Z9tP4>
.
|
|
Thanks really nice! Make sure you pull the latest code first before sending next PR, unlike this PR had alot of commits you look. Bounty is available for issues here : bounty
I would request you to look into this : #396 as its the one of the most important bug waiting for PR |
shanalikhan
pushed a commit
that referenced
this pull request
Feb 15, 2019
* Addapt test to changes. * Get tabs or spaces and line breaks for each ignored line. * Remove unnecesary tests * New Approach for parsing pragmas Instead of replacing entires blocks, we will loop each line in the settings file. For each line we are checking if should be ignored or if comments must be toggled. There is a block of code that is repeated to ensure the readability of the code. * Support for Brackets and unifying repeated code * Test for multi-line settings supporting curly braces and brackets. Changing test text extension. * Remove unused function. Add new lines after ignored settings block.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Short description of what this resolves: Fix #701
Fixes: #701
NOTE: This adds support for multi-line settings with code sync pragma. It only support one level of nested settings for now.
Usage of pragmas in nested settings: