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

Change wrong nb of parameters error messages to display function name #641

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

Merudo
Copy link
Member

@Merudo Merudo commented Sep 1, 2019

Old error message:

Invalid number of parameters 0, expected exactly 1 parameter(s)

New error message:

Function 'input' requires exactly 1 parameters; 0 were provided.


This change is Reviewable

@Merudo
Copy link
Member Author

Merudo commented Sep 1, 2019

Before RPTools/parser#3 gets merged, you can test out this branch by getting it, then changing in build.gradle the line

implementation 'net.rptools.parser:parser:1.4.0.+'

to

implementation 'com.github.Merudo:parser:1.5.5'

@Merudo Merudo force-pushed the parserErrors branch 2 times, most recently from e529298 to a823be9 Compare September 5, 2019 06:32
- Change wrong number of parameters error messages to always display name of function
- Builds on RPTools/parser#3 to work
- Close RPTools#629
@Merudo
Copy link
Member Author

Merudo commented Sep 6, 2019

@cwisniew approved the parser merge, so this works fine now.

Copy link
Contributor

@Phergus Phergus left a comment

Choose a reason for hiding this comment

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

Minor concern about the use of "master-SNAPSHOT" in the build.gradle file as we haven't done that in the past. It has been nice to just see at a glance what version of a library was being pulled in.

Reviewed 6 of 6 files at r1.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@Merudo
Copy link
Member Author

Merudo commented Sep 6, 2019

I'll ask @cwisniew to release a new version of the parser, so it will be cleaner.

@Phergus
Copy link
Contributor

Phergus commented Sep 6, 2019

It may be that the way you did it is more flexible for future builds. @cwisniew ? @JamzTheMan ?

@JamzTheMan
Copy link
Member

I created a 1.5.4 release and is now available, give it a try.

@Phergus All we need to do is a GitHub "Release" off master for Parser. JitPack does the rest and you can vew them here: https://jitpack.io/#RPTools/parser/1.5.4

It's actually pretty easy. There's no CI right now for Parser. And ya, we should only use MASTER--Snapshot for testing. Since @cwisniew approved it I assume we are all good to go.

@JamzTheMan JamzTheMan added 1 feature Adding functionality that adds value low Low priority bug/enhancement labels Sep 6, 2019
@Phergus Phergus merged commit d5c2653 into RPTools:develop Sep 6, 2019
@Phergus
Copy link
Contributor

Phergus commented Sep 6, 2019

Roger that. Merging on it then.

@JamzTheMan
Copy link
Member

JamzTheMan commented Sep 6, 2019

Oh oh, um, sorry, guess I wasn't clear. @Merudo (or anyone) still need to update the gradle file here to point to the parser-1.5.4 release vs master snapshot...

I only did a "Parser" release which created a new Parser release to be used... Another PR will need to be created to update the gradle.build in MapTool

@Merudo
Copy link
Member Author

Merudo commented Sep 6, 2019

PR #659 changes the parser to 1.5.4

@Phergus
Copy link
Contributor

Phergus commented Sep 6, 2019

My bad. I misread your post and didn't catch the "for testing".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Adding functionality that adds value low Low priority bug/enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants