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

unexpected WARN while using `with-resource-id`, lift-3.0-SNAPSHOT #1521

Merged
merged 2 commits into from Apr 7, 2014

Conversation

Projects
None yet
4 participants
@Shadowfiend
Copy link
Member

Shadowfiend commented Mar 15, 2014

Mailing list topic: https://groups.google.com/forum/#!msg/liftweb/z5rKAxR0y08/S811GG3BLmsJ

The actual bug is with having a WARNing:
01:50:52.564 WARN [qtp1170205978-851126 - /img/favicon.png?F1171910628180AB1FAH=_] net.liftweb.http.LiftRules - Unmapped Lift-like parameter seen in request [/img/favicon.png]: F1171910628180AB1FAH

Shadowfiend added some commits Mar 15, 2014

Rename pageResourceId to instanceResourceId in LiftRules.
Also drop a comment on it indicating what it’s for, and move it
closer to its usage spot.
Add instance- prefix to instance resource id.
This makes it clearer what it is when looking at the output of
lift:with-reosurce-id (before it looked like a function binding),
and also ensures that we won’t warn you about an unbound
function name when using with-resource-id.
@Shadowfiend

This comment has been minimized.

Copy link
Member

Shadowfiend commented Mar 29, 2014

Any feedback here?

@vn971

This comment has been minimized.

Copy link
Author

vn971 commented Mar 29, 2014

Ah, sorry, I forgot about the PR/issue, only replied in the mailing list. The issue can be closed. Thank you very much!

@vn971 vn971 closed this Mar 29, 2014

@Shadowfiend

This comment has been minimized.

Copy link
Member

Shadowfiend commented Mar 29, 2014

Heh, um, no, the functionality still needs to land in the repository. I was asking about feedback regarding the code, from other committers ;)

@Shadowfiend Shadowfiend reopened this Mar 29, 2014

@fmpwizard fmpwizard added this to the 2.6-M3 milestone Mar 29, 2014

@fmpwizard

This comment has been minimized.

Copy link
Member

fmpwizard commented Mar 29, 2014

👍

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Apr 4, 2014

Curious: Is there some spec we could add to demonstrate the fixed behavior or no?

@Shadowfiend

This comment has been minimized.

Copy link
Member

Shadowfiend commented Apr 5, 2014

Not that I can think of, no. Partly because it's not “fixed” so much as “different”. It's not really a bug that it was behaving as it was, just a slight change of behavior to reflect the way the unmapped parameter warnings work. I mean, we could write a spec that attachResourceId adds a resource that is prefixed by instance-, but that seems mostly tautological, and not having that prefix doesn't break anything. We could assert that it doesn't start with F, but that's a weird assertion to make since the only reason we don't want it to do that is because of the interaction with the warning bits. And we could write an integration test that asserts we don't get a log message somehow, but that seems like excessive effort for a relatively minor inconvenience rather than a real bug.

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Apr 7, 2014

Alright. I'm down. Gonna double check that compile and those tests then party.

@farmdawgnation

This comment has been minimized.

Copy link
Member

farmdawgnation commented Apr 7, 2014

ship

farmdawgnation added a commit that referenced this pull request Apr 7, 2014

Merge pull request #1521 from lift/asc-issue-1521
Fix the unexpected WARN while using `with-resource-id` by making instance IDs their own thing.

@farmdawgnation farmdawgnation merged commit 0b4c96f into master Apr 7, 2014

@farmdawgnation farmdawgnation deleted the asc-issue-1521 branch Apr 7, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment