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.

Sign up for GitHub

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

Jump to bottom

Ignore JSON formatting in command-line tests #13552

Closed
cameel opened this issue Sep 22, 2022 · 7 comments
Closed

Ignore JSON formatting in command-line tests #13552

cameel opened this issue Sep 22, 2022 · 7 comments
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. nice to have We don’t see a good reason not to have it but won’t go out of our way to implement it. stale The issue/PR was marked as stale because it has been open for too long. testing 🔨

Comments

@cameel
Copy link
Member

cameel commented Sep 22, 2022

Related to #13544 and #7665.

Currently command-line tests in StandardJSON mode expect output exactly matching the expectations. This is not a problem until we try to post-process the output and run into problems like #13544.

Since the formatting is irrelevant most of the time, I think cmdlineTests.sh should be using jq on the output and expectations to reformat them before diffing.

We'll also need a way to disable this to keep tests for the --pretty-print CLI option functional. I'd go with that I suggested in #7665, i.e. disable this mode when a file called no-pretty-print exists in the test directory.
@cameel cameel added good first issue testing 🔨 low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. nice to have We don’t see a good reason not to have it but won’t go out of our way to implement it. labels Sep 22, 2022
@cameel cameel mentioned this issue Oct 20, 2022
Merged
@Krish-bhardwaj
Copy link

Hi @cameel i have been doing open source contribution for a long time and i have worked on solidity language , i would like to push my self to solve some more complex problem in Open source projects like this if you can guide and assign me this issue it would be really help full
Thanks in advance

@cameel
Copy link
Member Author

cameel commented Oct 26, 2022

Maybe you could drop by at the #solidity-dev channel to discuss this first? This is an easy issue but it's also very low priority so I'd rather find something more relevant for you. But this depends on how comfortable you are with C++ so we should talk a bit first.

@Krish-bhardwaj
Copy link

Yes sir I am comfortable in cpp I have been doing competitive programming for a long time and I have used cpp over there rest it will be better for me if I solve a low priority issue first as this can give me some time to learn new this also rest it's up to you if you can guide me I would take up this issue and solve it.

@cameel
Copy link
Member Author

cameel commented Oct 27, 2022 •
edited
Loading

The problem with low priority issues though is that we get so many PRs solving them that we're currently struggling to get through the backlog (65 open PRs now, down from ~100 not so long ago) and they're taking the team's attention away from the real stuff we should be working on. We do appreciate contributions, but I'd rather find you something related to our current roadmap than this random issue :) Especially that the work to do here is pretty much all Bash, not C++.

If you're good at C++, that's great! We're really in need of more skilled contributors who can take on tasks that are not just simple refactors.

@cameel
Copy link
Member Author

cameel commented Oct 27, 2022

@Krish-bhardwaj How about #13652?

@NunoFilipeSantos NunoFilipeSantos added the good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. label Dec 5, 2022
@github-actions
Copy link

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Mar 31, 2023
@github-actions
Copy link

github-actions bot commented Apr 8, 2023

Hi everyone! This issue has been automatically closed due to inactivity.
If you think this issue is still relevant in the latest Solidity version and you have something to contribute, feel free to reopen.
However, unless the issue is a concrete proposal that can be implemented, we recommend starting a language discussion on the forum instead.

@github-actions github-actions bot added the closed due inactivity The issue/PR was automatically closed due to inactivity. label Apr 8, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Apr 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
closed due inactivity The issue/PR was automatically closed due to inactivity. good first issue candidate Could be a "good first issue" but something is blocking it or it has open questions. low effort There is not much implementation work to be done. The task is very easy or tiny. low impact Changes are not very noticeable or potential benefits are limited. nice to have We don’t see a good reason not to have it but won’t go out of our way to implement it. stale The issue/PR was marked as stale because it has been open for too long. testing 🔨
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@cameel @NunoFilipeSantos @Krish-bhardwaj