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

Updating the apple raster function #3

Merged
merged 6 commits into from
Dec 28, 2022
Merged

Updating the apple raster function #3

merged 6 commits into from
Dec 28, 2022

Conversation

Kappuccino111
Copy link
Contributor

@Kappuccino111 Kappuccino111 commented Dec 22, 2022

@tillkamppeter here is the implementation to support any no. of resolutions in the apple raster.
Have written code that parses through the pointers to find the total list of resolutions and then determines the best possible resolutions just as discussed.

I have also written an email to you corresponding the update.

Updating the apple raster function to support any no. of resolutions.
@tillkamppeter
Copy link
Member

Could you also check the coding style and indentation? A tab in our code is always equivalent with 8 spaces. We indent 2 spaces per { ... } level, an if, while or similar should look like

if (...)
{
  ...
}

Simply check whether your code matches with the code of the rest of the file.

@Kappuccino111
Copy link
Contributor Author

@tillkamppeter I have updated the indentation and now it follows the 2-indent space guide and also updated the if condition.

@tillkamppeter
Copy link
Member

Thanks for correcting the indentation.

Could you also check the spaces around operators? I have marked some examples, but please check all your code, there are probably more.

@Kappuccino111
Copy link
Contributor Author

Kappuccino111 commented Dec 24, 2022

@tillkamppeter I have updated the operator, if and while indents and
have also checked for other operators and indents in my code to the best of my best knowledge.

@Kappuccino111
Copy link
Contributor Author

@tillkamppeter Eagerly awaiting further contribution.

@tillkamppeter
Copy link
Member

If I look at the complete file in your repository there are still problems with indentation.

Are you actually using 8-char tabs?

And also it should NOT be

if (...)
  {
    ...
  }

but correct is

if (...)
{
  ...
}

Could you correct?

@Kappuccino111
Copy link
Contributor Author

Kappuccino111 commented Dec 28, 2022

@tillkamppeter I have rechecked the file for indents and operator spaces.
I guess the problem was in the git editor layout.....since the indents were shown properly in VS Code .....I have now edited according to the git editor layout.
Could you please check and confirm ?

@tillkamppeter
Copy link
Member

I checked

https://github.com/Kappuccino111/libppd/blob/master/ppd/ppd-generator.c

again.

Could you please correct

  • Indentation on lines: 654, 655, 664, 670, 676
  • Comment spacing: A space between code line end and // and a space between // and comment text, line: 611, 621, 622, 661

Could you correct these?

@Kappuccino111
Copy link
Contributor Author

Kappuccino111 commented Dec 28, 2022

  • Comment spacing: A space between code line end and // and a space between // and comment text, line: 611, 621, 622, 661

@tillkamppeter for comment spacing should I just make a multi line comment and add that before the code part.

Also the code I have on VS Code implements all the indentation aspects ..... I guess I should check the file from the git editor itself........

@tillkamppeter
Copy link
Member

no, simply add the missing spaces in the mentioned lines, there must be one before and one after the //: xxx;//yyy -> xxx; // yyy

@Kappuccino111
Copy link
Contributor Author

@tillkamppeter Have updated the requirements...... I would make sure that I further adhere to these while coding in the near future.

@tillkamppeter tillkamppeter merged commit 0b2ca66 into OpenPrinting:master Dec 28, 2022
@Kappuccino111
Copy link
Contributor Author

@tillkamppeter Thank you for your guidance. Eagerly awaiting further contribution..👍

@Kappuccino111
Copy link
Contributor Author

@tillkamppeter Which issue shall I start working on now ?.... Really excited 🔥 ! .... Again thank you for the guidance....

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.

2 participants